From b828c56dc926dc47029834235c08e6b876f53e7f Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Sat, 31 Aug 2024 22:05:42 +0200 Subject: [PATCH 01/38] [ticket/17414] Start work on new captcha classes PHPBB-17414 --- .../captcha/plugins/plugin_interface.php | 42 +++++++++ phpBB/phpbb/captcha/plugins/turnstile.php | 93 +++++++++++++++++++ .../foo/test_captcha/captcha/test_captcha.php | 33 +++++++ .../ext/foo/test_captcha/composer.json | 23 +++++ .../ext/foo/test_captcha/config/services.yml | 5 + 5 files changed, 196 insertions(+) create mode 100644 phpBB/phpbb/captcha/plugins/plugin_interface.php create mode 100644 phpBB/phpbb/captcha/plugins/turnstile.php create mode 100644 tests/functional/fixtures/ext/foo/test_captcha/captcha/test_captcha.php create mode 100644 tests/functional/fixtures/ext/foo/test_captcha/composer.json create mode 100644 tests/functional/fixtures/ext/foo/test_captcha/config/services.yml diff --git a/phpBB/phpbb/captcha/plugins/plugin_interface.php b/phpBB/phpbb/captcha/plugins/plugin_interface.php new file mode 100644 index 0000000000..f40745db21 --- /dev/null +++ b/phpBB/phpbb/captcha/plugins/plugin_interface.php @@ -0,0 +1,42 @@ +config = $config; + } + + public function is_available() + { + return ($this->config->offsetGet('captcha_turnstile_key') ?? false); + } + + public function get_template() + { + return 'custom_captcha.html'; // Template file for displaying the CAPTCHA + } + + public function execute() + { + // Perform any necessary initialization or setup + } + + public function validate() + { + // Implement server-side validation logic here + // Example: Validate the submitted CAPTCHA value using Cloudflare API + + // Your Cloudflare API credentials + $api_email = 'your_email@example.com'; + $api_key = 'your_api_key'; + + // Cloudflare API endpoint for CAPTCHA verification + $endpoint = 'https://api.cloudflare.com/client/v4/captcha/validate'; + + // CAPTCHA data to be sent in the request + $data = [ + 'email' => $api_email, + 'key' => $api_key, + 'response' => $this->confirm_code + ]; + + // Initialize cURL session + $ch = curl_init(); + + // Set cURL options + curl_setopt_array($ch, [ + CURLOPT_URL => $endpoint, + CURLOPT_POST => true, + CURLOPT_POSTFIELDS => json_encode($data), + CURLOPT_HTTPHEADER => [ + 'Content-Type: application/json', + 'Accept: application/json' + ], + CURLOPT_RETURNTRANSFER => true + ]); + + // Execute the cURL request + $response = curl_exec($ch); + + // Check for errors + if ($response === false) { + // Handle cURL error + curl_close($ch); + return false; + } + + // Decode the JSON response + $result = json_decode($response, true); + + // Check if the response indicates success + if (isset($result['success']) && $result['success'] === true) { + // CAPTCHA validation passed + curl_close($ch); + return true; + } else { + // CAPTCHA validation failed + curl_close($ch); + return false; + } + } + + public function get_generator_class() + { + throw new \Exception('No generator class given.'); + } +} \ No newline at end of file diff --git a/tests/functional/fixtures/ext/foo/test_captcha/captcha/test_captcha.php b/tests/functional/fixtures/ext/foo/test_captcha/captcha/test_captcha.php new file mode 100644 index 0000000000..fb8c6ed090 --- /dev/null +++ b/tests/functional/fixtures/ext/foo/test_captcha/captcha/test_captcha.php @@ -0,0 +1,33 @@ +=8.1" + }, + "extra": { + "display-name": "phpBB 4.0 Test Captcha", + "soft-require": { + "phpbb/phpbb": "4.0.*@dev" + } + } +} diff --git a/tests/functional/fixtures/ext/foo/test_captcha/config/services.yml b/tests/functional/fixtures/ext/foo/test_captcha/config/services.yml new file mode 100644 index 0000000000..75cdf52270 --- /dev/null +++ b/tests/functional/fixtures/ext/foo/test_captcha/config/services.yml @@ -0,0 +1,5 @@ +services: + foo_test_captcha.captcha.test_captcha: + class: foo\test_captcha\captcha\test_captcha + tags: + - { name: captcha.plugins } From 1b8918448975b923fac78445ecfa5fe6e9dfe554 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Thu, 3 Oct 2024 21:18:14 +0200 Subject: [PATCH 02/38] [ticket/17415] Start adding legacy wrapper for old captchas PHPBB-17415 --- .../phpbb/captcha/plugins/legacy_wrapper.php | 82 +++++++++++++++++++ .../captcha/plugins/plugin_interface.php | 13 ++- phpBB/phpbb/captcha/plugins/turnstile.php | 11 +++ 3 files changed, 105 insertions(+), 1 deletion(-) create mode 100644 phpBB/phpbb/captcha/plugins/legacy_wrapper.php diff --git a/phpBB/phpbb/captcha/plugins/legacy_wrapper.php b/phpBB/phpbb/captcha/plugins/legacy_wrapper.php new file mode 100644 index 0000000000..90ff01995b --- /dev/null +++ b/phpBB/phpbb/captcha/plugins/legacy_wrapper.php @@ -0,0 +1,82 @@ + + * @license GNU General Public License, version 2 (GPL-2.0) + * + * For full copyright and license information, please see + * the docs/CREDITS.txt file. + * + */ + +namespace phpbb\captcha\plugins; + +class legacy_wrapper implements plugin_interface +{ + private $legacy_captcha; + + public function __construct($legacy_captcha) + { + $this->legacy_captcha = $legacy_captcha; + } + + /** + * Check if the plugin is available + * @return bool True if the plugin is available, false if not + */ + public function is_available(): bool + { + if (method_exists($this->legacy_captcha, 'is_available')) + { + return $this->legacy_captcha->is_available(); + } + + return false; + } + + /** + * Check if the plugin has a configuration + * + * @return bool True if the plugin has a configuration, false if not + */ + public function has_config(): bool + { + if (method_exists($this->legacy_captcha, 'has_config')) + { + return $this->legacy_captcha->has_config(); + } + + return false; + } + + /** + * Get the name of the plugin, should be language variable + * + * @return string + */ + public function get_name(): string + { + if (method_exists($this->legacy_captcha, 'get_name')) + { + return $this->legacy_captcha->has_config(); + } + + return false; + } + + /** + * Display the captcha for the specified type + * + * @param int $type Type of captcha, should be one of the CONFIRMATION_* constants + * @return void + */ + public function show(int $type): void + { + if (method_exists($this->legacy_captcha, 'init')) + { + $this->legacy_captcha->init($type); + } + } +} \ No newline at end of file diff --git a/phpBB/phpbb/captcha/plugins/plugin_interface.php b/phpBB/phpbb/captcha/plugins/plugin_interface.php index f40745db21..51c8d3d23d 100644 --- a/phpBB/phpbb/captcha/plugins/plugin_interface.php +++ b/phpBB/phpbb/captcha/plugins/plugin_interface.php @@ -1,4 +1,15 @@ + * @license GNU General Public License, version 2 (GPL-2.0) + * + * For full copyright and license information, please see + * the docs/CREDITS.txt file. + * + */ namespace phpbb\captcha\plugins; @@ -30,7 +41,7 @@ interface plugin_interface * * @return string */ - public static function get_name(): string; + public function get_name(): string; /** * Display the captcha for the specified type diff --git a/phpBB/phpbb/captcha/plugins/turnstile.php b/phpBB/phpbb/captcha/plugins/turnstile.php index 1388f33012..130949e708 100644 --- a/phpBB/phpbb/captcha/plugins/turnstile.php +++ b/phpBB/phpbb/captcha/plugins/turnstile.php @@ -1,4 +1,15 @@ + * @license GNU General Public License, version 2 (GPL-2.0) + * + * For full copyright and license information, please see + * the docs/CREDITS.txt file. + * + */ namespace phpbb\captcha\plugins; From 15b51e3b8f8f9aeeeecf6522d83377233e909060 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Fri, 4 Oct 2024 13:42:18 +0200 Subject: [PATCH 03/38] [ticket/17414] Deprecate captcha constants PHPBB-17414 --- phpBB/includes/constants.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/phpBB/includes/constants.php b/phpBB/includes/constants.php index 3304194cf4..0ef409767d 100644 --- a/phpBB/includes/constants.php +++ b/phpBB/includes/constants.php @@ -152,9 +152,13 @@ define('FULL_FOLDER_DELETE', -2); define('FULL_FOLDER_HOLD', -1); // Confirm types +/** @deprecated 4.0.0-a1 Replaced by \phpbb\captcha\plugins\plugin_interface::CONFIRM_REGISTRATION, to be removed in 5.0.0-a1 */ define('CONFIRM_REG', 1); +/** @deprecated 4.0.0-a1 Replaced by \phpbb\captcha\plugins\plugin_interface::CONFIRM_LOGIN, to be removed in 5.0.0-a1 */ define('CONFIRM_LOGIN', 2); +/** @deprecated 4.0.0-a1 Replaced by \phpbb\captcha\plugins\plugin_interface::CONFIRM_POST, to be removed in 5.0.0-a1 */ define('CONFIRM_POST', 3); +/** @deprecated 4.0.0-a1 Replaced by \phpbb\captcha\plugins\plugin_interface::CONFIRM_REPORT, to be removed in 5.0.0-a1 */ define('CONFIRM_REPORT', 4); // Categories - Attachments From e84e9cace4e10b2a6d5a759d9ab82ffc3e313521 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Fri, 4 Oct 2024 13:42:45 +0200 Subject: [PATCH 04/38] [ticket/17414] Extend plugin interface and wrapper PHPBB-17414 --- phpBB/includes/ucp/ucp_register.php | 8 ++- phpBB/phpbb/captcha/factory.php | 15 ++++- .../phpbb/captcha/plugins/legacy_wrapper.php | 64 ++++++++++++++++++- .../captcha/plugins/plugin_interface.php | 40 ++++++++++-- phpBB/posting.php | 8 ++- 5 files changed, 119 insertions(+), 16 deletions(-) diff --git a/phpBB/includes/ucp/ucp_register.php b/phpBB/includes/ucp/ucp_register.php index 40b14a9b54..8ad37047cb 100644 --- a/phpBB/includes/ucp/ucp_register.php +++ b/phpBB/includes/ucp/ucp_register.php @@ -235,8 +235,10 @@ class ucp_register // The CAPTCHA kicks in here. We can't help that the information gets lost on language change. if ($config['enable_confirm']) { - $captcha = $phpbb_container->get('captcha.factory')->get_instance($config['captcha_plugin']); - $captcha->init(CONFIRM_REG); + /** @var \phpbb\captcha\factory $captcha_factory */ + $captcha_factory = $phpbb_container->get('captcha.factory'); + $captcha = $captcha_factory->get_instance($config['captcha_plugin']); + $captcha->init(\phpbb\captcha\plugins\plugin_interface::CONFIRM_REGISTRATION); } $timezone = $config['board_timezone']; @@ -426,7 +428,7 @@ class ucp_register } // Okay, captcha, your job is done. - if ($config['enable_confirm'] && isset($captcha)) + if ($config['enable_confirm']) { $captcha->reset(); } diff --git a/phpBB/phpbb/captcha/factory.php b/phpBB/phpbb/captcha/factory.php index 2e75ce8667..971a7b2118 100644 --- a/phpBB/phpbb/captcha/factory.php +++ b/phpBB/phpbb/captcha/factory.php @@ -13,6 +13,9 @@ namespace phpbb\captcha; +use phpbb\captcha\plugins\legacy_wrapper; +use phpbb\captcha\plugins\plugin_interface; + class factory { /** @@ -41,11 +44,17 @@ class factory * Return a new instance of a given plugin * * @param $name - * @return object|null + * @return plugin_interface */ - public function get_instance($name) + public function get_instance($name): plugin_interface { - return $this->container->get($name); + $captcha = $this->container->get($name); + if ($captcha instanceof plugin_interface) + { + return $captcha; + } + + return new legacy_wrapper($name); } /** diff --git a/phpBB/phpbb/captcha/plugins/legacy_wrapper.php b/phpBB/phpbb/captcha/plugins/legacy_wrapper.php index 90ff01995b..bceb771800 100644 --- a/phpBB/phpbb/captcha/plugins/legacy_wrapper.php +++ b/phpBB/phpbb/captcha/plugins/legacy_wrapper.php @@ -17,6 +17,8 @@ class legacy_wrapper implements plugin_interface { private $legacy_captcha; + private string $last_error; + public function __construct($legacy_captcha) { $this->legacy_captcha = $legacy_captcha; @@ -72,11 +74,69 @@ class legacy_wrapper implements plugin_interface * @param int $type Type of captcha, should be one of the CONFIRMATION_* constants * @return void */ - public function show(int $type): void + public function init(int $type): void { if (method_exists($this->legacy_captcha, 'init')) { $this->legacy_captcha->init($type); } } -} \ No newline at end of file + + /** + * {@inheritDoc} + */ + public function validate(array $request_data): bool + { + if (method_exists($this->legacy_captcha, 'validate')) + { + $error = $this->legacy_captcha->validate($request_data); + if ($error) + { + $this->last_error = $error; + return false; + } + + return true; + } + + return false; + } + + /** + * {@inheritDoc} + */ + public function is_solved(): bool + { + if (method_exists($this->legacy_captcha, 'is_solved')) + { + return $this->legacy_captcha->is_solved(); + } + + return false; + } + + /** + * {@inheritDoc} + */ + public function reset(): void + { + if (method_exists($this->legacy_captcha, 'reset')) + { + $this->legacy_captcha->reset(); + } + } + + /** + * {@inheritDoc} + */ + public function get_attempt_count(): int + { + if (method_exists($this->legacy_captcha, 'get_attempt_count')) + { + return $this->legacy_captcha->get_attempt_count(); + } + + // Ensure this is deemed as too many attempts + return PHP_INT_MAX; + } +} diff --git a/phpBB/phpbb/captcha/plugins/plugin_interface.php b/phpBB/phpbb/captcha/plugins/plugin_interface.php index 51c8d3d23d..9d0b0cb4de 100644 --- a/phpBB/phpbb/captcha/plugins/plugin_interface.php +++ b/phpBB/phpbb/captcha/plugins/plugin_interface.php @@ -15,16 +15,17 @@ namespace phpbb\captcha\plugins; interface plugin_interface { - const CONFIRMATION_REGISTRATION = 1; - const CONFIRMATION_LOGIN = 2; + const CONFIRM_REGISTRATION = 1; + const CONFIRM_LOGIN = 2; - const CONFIRMATION_POST = 3; + const CONFIRM_POST = 3; - const CONFIRMATION_REPORT = 4; + const CONFIRM_REPORT = 4; /** * Check if the plugin is available + * * @return bool True if the plugin is available, false if not */ public function is_available(): bool; @@ -49,5 +50,34 @@ interface plugin_interface * @param int $type Type of captcha, should be one of the CONFIRMATION_* constants * @return void */ - public function show(int $type): void; + public function init(int $type): void; + + /** + * Validate the captcha with the given request data + * + * @param array $request_data Request data for the captcha + * @return bool True if request data was valid captcha reply, false if not + */ + public function validate(array $request_data): bool; + + /** + * Return whether captcha was solved + * + * @return bool True if captcha was solved, false if not + */ + public function is_solved(): bool; + + /** + * Reset captcha state, e.g. after checking if it's valid + * + * @return void + */ + public function reset(): void; + + /** + * Get attempt count for this captcha and user + * + * @return int Number of attempts + */ + public function get_attempt_count(): int; } diff --git a/phpBB/posting.php b/phpBB/posting.php index 63ca4fef97..988b7f03e2 100644 --- a/phpBB/posting.php +++ b/phpBB/posting.php @@ -455,8 +455,10 @@ if (!$is_authed || !empty($error)) if ($config['enable_post_confirm'] && !$user->data['is_registered']) { - $captcha = $phpbb_container->get('captcha.factory')->get_instance($config['captcha_plugin']); - $captcha->init(CONFIRM_POST); + /** @var \phpbb\captcha\factory $captcha_factory */ + $captcha_factory = $phpbb_container->get('captcha.factory'); + $captcha = $captcha_factory->get_instance($config['captcha_plugin']); + $captcha->init(\phpbb\captcha\plugins\plugin_interface::CONFIRM_POST); } // Is the user able to post within this forum? @@ -1600,7 +1602,7 @@ if ($submit || $preview || $refresh) ); extract($phpbb_dispatcher->trigger_event('core.posting_modify_submit_post_after', compact($vars))); - if ($config['enable_post_confirm'] && !$user->data['is_registered'] && (isset($captcha) && $captcha->is_solved() === true) && ($mode == 'post' || $mode == 'reply' || $mode == 'quote')) + if ($config['enable_post_confirm'] && !$user->data['is_registered'] && $captcha->is_solved() === true && ($mode == 'post' || $mode == 'reply' || $mode == 'quote')) { $captcha->reset(); } From ae9f97b7e2dd8b75b23d62c8ee26ab99d96355f6 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Sat, 5 Oct 2024 14:17:40 +0200 Subject: [PATCH 05/38] [ticket/17415] Move install step of Q&A to migration PHPBB-17415 --- phpBB/phpbb/captcha/plugins/qa.php | 50 ----------- .../db/migration/data/v400/qa_captcha.php | 89 +++++++++++++++++++ 2 files changed, 89 insertions(+), 50 deletions(-) create mode 100644 phpBB/phpbb/db/migration/data/v400/qa_captcha.php diff --git a/phpBB/phpbb/captcha/plugins/qa.php b/phpBB/phpbb/captcha/plugins/qa.php index b9bfac33f1..106cedb7c2 100644 --- a/phpBB/phpbb/captcha/plugins/qa.php +++ b/phpBB/phpbb/captcha/plugins/qa.php @@ -336,56 +336,6 @@ class qa */ function install() { - global $phpbb_container; - - $db_tool = $phpbb_container->get('dbal.tools'); - $schemas = array( - $this->table_captcha_questions => array ( - 'COLUMNS' => array( - 'question_id' => array('UINT', null, 'auto_increment'), - 'strict' => array('BOOL', 0), - 'lang_id' => array('UINT', 0), - 'lang_iso' => array('VCHAR:30', ''), - 'question_text' => array('TEXT_UNI', ''), - ), - 'PRIMARY_KEY' => 'question_id', - 'KEYS' => array( - 'lang' => array('INDEX', 'lang_iso'), - ), - ), - $this->table_captcha_answers => array ( - 'COLUMNS' => array( - 'question_id' => array('UINT', 0), - 'answer_text' => array('STEXT_UNI', ''), - ), - 'KEYS' => array( - 'qid' => array('INDEX', 'question_id'), - ), - ), - $this->table_qa_confirm => array ( - 'COLUMNS' => array( - 'session_id' => array('CHAR:32', ''), - 'confirm_id' => array('CHAR:32', ''), - 'lang_iso' => array('VCHAR:30', ''), - 'question_id' => array('UINT', 0), - 'attempts' => array('UINT', 0), - 'confirm_type' => array('USINT', 0), - ), - 'KEYS' => array( - 'session_id' => array('INDEX', 'session_id'), - 'lookup' => array('INDEX', array('confirm_id', 'session_id', 'lang_iso')), - ), - 'PRIMARY_KEY' => 'confirm_id', - ), - ); - - foreach ($schemas as $table => $schema) - { - if (!$db_tool->sql_table_exists($table)) - { - $db_tool->sql_create_table($table, $schema); - } - } } /** diff --git a/phpBB/phpbb/db/migration/data/v400/qa_captcha.php b/phpBB/phpbb/db/migration/data/v400/qa_captcha.php new file mode 100644 index 0000000000..69cca2dc15 --- /dev/null +++ b/phpBB/phpbb/db/migration/data/v400/qa_captcha.php @@ -0,0 +1,89 @@ + + * @license GNU General Public License, version 2 (GPL-2.0) + * + * For full copyright and license information, please see + * the docs/CREDITS.txt file. + * + */ + +namespace phpbb\db\migration\data\v400; + +use phpbb\db\migration\migration; + +class qa_captcha extends migration +{ + public function effectively_installed(): bool + { + return $this->db_tools->sql_table_exists($this->tables['captcha_qa_questions']) + && $this->db_tools->sql_table_exists($this->tables['captcha_qa_answers']) + && $this->db_tools->sql_table_exists($this->tables['captcha_qa_confirm']); + } + + public static function depends_on(): array + { + return [ + '\phpbb\db\migration\data\v400\dev', + ]; + } + + public function update_schema(): array + { + return [ + 'add_tables' => [ + $this->tables['captcha_qa_questions'] => [ + 'COLUMNS' => [ + 'question_id' => ['UINT', null, 'auto_increment'], + 'strict' => ['BOOL', 0], + 'lang_id' => ['UINT', 0], + 'lang_iso' => ['VCHAR:30', ''], + 'question_text' => ['TEXT_UNI', ''], + ], + 'PRIMARY_KEY' => 'question_id', + 'KEYS' => [ + 'lang' => ['INDEX', 'lang_iso'], + ], + ], + $this->tables['captcha_qa_answers'] => [ + 'COLUMNS' => [ + 'question_id' => ['UINT', 0], + 'answer_text' => ['STEXT_UNI', ''], + ], + 'KEYS' => [ + 'qid' => ['INDEX', 'question_id'], + ], + ], + $this->tables['captcha_qa_confirm'] => [ + 'COLUMNS' => [ + 'session_id' => ['CHAR:32', ''], + 'confirm_id' => ['CHAR:32', ''], + 'lang_iso' => ['VCHAR:30', ''], + 'question_id' => ['UINT', 0], + 'attempts' => ['UINT', 0], + 'confirm_type' => ['USINT', 0], + ], + 'KEYS' => [ + 'session_id' => ['INDEX', 'session_id'], + 'lookup' => ['INDEX', ['confirm_id', 'session_id', 'lang_iso']], + ], + 'PRIMARY_KEY' => 'confirm_id', + ], + ], + ]; + } + + public function revert_schema(): array + { + return [ + 'drop_tables' => [ + $this->tables['captcha_qa_questions'], + $this->tables['captcha_qa_answers'], + $this->tables['captcha_qa_confirm'] + ], + ]; + } +} From 89bb72c277cf2c10a1f58182fdd0da962752c874 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Sat, 5 Oct 2024 17:05:38 +0200 Subject: [PATCH 06/38] [ticket/17415] Correctly wrap legacy captchas PHPBB-17415 --- phpBB/phpbb/captcha/factory.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/phpBB/phpbb/captcha/factory.php b/phpBB/phpbb/captcha/factory.php index 971a7b2118..d9259d8da9 100644 --- a/phpBB/phpbb/captcha/factory.php +++ b/phpBB/phpbb/captcha/factory.php @@ -54,7 +54,7 @@ class factory return $captcha; } - return new legacy_wrapper($name); + return new legacy_wrapper($captcha); } /** From 8429145241759a0966caf7f28085ddd59e63f4b0 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Sat, 5 Oct 2024 17:06:10 +0200 Subject: [PATCH 07/38] [ticket/17414] Remove install steps for captcha plugins This should be handled by migrations instead. PHPBB-17414 --- phpBB/includes/acp/acp_captcha.php | 4 +- .../captcha/plugins/captcha_abstract.php | 5 -- .../phpbb/captcha/plugins/legacy_wrapper.php | 48 ++++++++++++++----- .../captcha/plugins/plugin_interface.php | 17 +++++++ phpBB/phpbb/captcha/plugins/qa.php | 12 ----- phpBB/phpbb/captcha/plugins/recaptcha.php | 5 -- 6 files changed, 54 insertions(+), 37 deletions(-) diff --git a/phpBB/includes/acp/acp_captcha.php b/phpBB/includes/acp/acp_captcha.php index ff9f515d32..69e13463ab 100644 --- a/phpBB/includes/acp/acp_captcha.php +++ b/phpBB/includes/acp/acp_captcha.php @@ -128,11 +128,9 @@ class acp_captcha if (isset($captchas['available'][$selected])) { $old_captcha = $factory->get_instance($config['captcha_plugin']); - $old_captcha->uninstall(); + $old_captcha->garbage_collect(); $config->set('captcha_plugin', $selected); - $new_captcha = $factory->get_instance($config['captcha_plugin']); - $new_captcha->install(); $phpbb_log->add('admin', $user->data['user_id'], $user->ip, 'LOG_CONFIG_VISUAL'); } diff --git a/phpBB/phpbb/captcha/plugins/captcha_abstract.php b/phpBB/phpbb/captcha/plugins/captcha_abstract.php index 012e28c987..fb8ccc5a70 100644 --- a/phpBB/phpbb/captcha/plugins/captcha_abstract.php +++ b/phpBB/phpbb/captcha/plugins/captcha_abstract.php @@ -184,11 +184,6 @@ abstract class captcha_abstract $this->garbage_collect(0); } - function install() - { - return; - } - function validate() { global $user; diff --git a/phpBB/phpbb/captcha/plugins/legacy_wrapper.php b/phpBB/phpbb/captcha/plugins/legacy_wrapper.php index bceb771800..e57abbb967 100644 --- a/phpBB/phpbb/captcha/plugins/legacy_wrapper.php +++ b/phpBB/phpbb/captcha/plugins/legacy_wrapper.php @@ -25,8 +25,7 @@ class legacy_wrapper implements plugin_interface } /** - * Check if the plugin is available - * @return bool True if the plugin is available, false if not + * {@inheritDoc} */ public function is_available(): bool { @@ -39,9 +38,7 @@ class legacy_wrapper implements plugin_interface } /** - * Check if the plugin has a configuration - * - * @return bool True if the plugin has a configuration, false if not + * {@inheritDoc} */ public function has_config(): bool { @@ -54,9 +51,7 @@ class legacy_wrapper implements plugin_interface } /** - * Get the name of the plugin, should be language variable - * - * @return string + * {@inheritDoc} */ public function get_name(): string { @@ -69,10 +64,7 @@ class legacy_wrapper implements plugin_interface } /** - * Display the captcha for the specified type - * - * @param int $type Type of captcha, should be one of the CONFIRMATION_* constants - * @return void + * {@inheritDoc} */ public function init(int $type): void { @@ -139,4 +131,36 @@ class legacy_wrapper implements plugin_interface // Ensure this is deemed as too many attempts return PHP_INT_MAX; } + + /** + * {@inheritDoc} + */ + public function get_demo_template(): string + { + if (method_exists($this->legacy_captcha, 'get_demo_template')) + { + return $this->legacy_captcha->get_demo_template(0); + } + + return ''; + } + + /** + * {@inheritDoc} + */ + public function garbage_collect(int $confirm_type = 0): void + { + if (method_exists($this->legacy_captcha, 'garbage_collect')) + { + $this->legacy_captcha->garbage_collect($confirm_type); + } + } + + public function acp_page($id, $module): void + { + if (method_exists($this->legacy_captcha, 'acp_page')) + { + $this->legacy_captcha->acp_page($id, $module); + } + } } diff --git a/phpBB/phpbb/captcha/plugins/plugin_interface.php b/phpBB/phpbb/captcha/plugins/plugin_interface.php index 9d0b0cb4de..41e825fad3 100644 --- a/phpBB/phpbb/captcha/plugins/plugin_interface.php +++ b/phpBB/phpbb/captcha/plugins/plugin_interface.php @@ -80,4 +80,21 @@ interface plugin_interface * @return int Number of attempts */ public function get_attempt_count(): int; + + /** + * Get template data for demo + * + * @return string Demo template file name + */ + public function get_demo_template(): string; + + /** + * Garbage collect captcha plugin + * + * @param int $confirm_type Confirm type to garbage collect, defaults to all (0) + * @return void + */ + public function garbage_collect(int $confirm_type = 0): void; + + public function acp_page($id, $module): void; } diff --git a/phpBB/phpbb/captcha/plugins/qa.php b/phpBB/phpbb/captcha/plugins/qa.php index 106cedb7c2..ffc67a7706 100644 --- a/phpBB/phpbb/captcha/plugins/qa.php +++ b/phpBB/phpbb/captcha/plugins/qa.php @@ -331,13 +331,6 @@ class qa $this->garbage_collect(0); } - /** - * API function - set up shop - */ - function install() - { - } - /** * API function - see what has to be done to validate */ @@ -597,11 +590,6 @@ class qa $user->add_lang('acp/board'); $user->add_lang('captcha_qa'); - if (!self::is_installed()) - { - $this->install(); - } - $module->tpl_name = 'captcha_qa_acp'; $module->page_title = 'ACP_VC_SETTINGS'; $form_key = 'acp_captcha'; diff --git a/phpBB/phpbb/captcha/plugins/recaptcha.php b/phpBB/phpbb/captcha/plugins/recaptcha.php index ef30baaa9b..a5dcf4efc6 100644 --- a/phpBB/phpbb/captcha/plugins/recaptcha.php +++ b/phpBB/phpbb/captcha/plugins/recaptcha.php @@ -184,11 +184,6 @@ class recaptcha extends captcha_abstract $this->garbage_collect(0); } - function install() - { - return; - } - function validate() { if (!parent::validate()) From 3676e895f3bbe9b96bd08a7ab7dfdf0126a8373f Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Sat, 5 Oct 2024 17:09:16 +0200 Subject: [PATCH 08/38] [ticket/17414] Remove uninstall method from captcha plugins PHPBB-17414 --- phpBB/phpbb/captcha/plugins/captcha_abstract.php | 5 ----- phpBB/phpbb/captcha/plugins/qa.php | 8 -------- phpBB/phpbb/captcha/plugins/recaptcha.php | 5 ----- 3 files changed, 18 deletions(-) diff --git a/phpBB/phpbb/captcha/plugins/captcha_abstract.php b/phpBB/phpbb/captcha/plugins/captcha_abstract.php index fb8ccc5a70..c6978a1bf1 100644 --- a/phpBB/phpbb/captcha/plugins/captcha_abstract.php +++ b/phpBB/phpbb/captcha/plugins/captcha_abstract.php @@ -179,11 +179,6 @@ abstract class captcha_abstract $db->sql_freeresult($result); } - function uninstall() - { - $this->garbage_collect(0); - } - function validate() { global $user; diff --git a/phpBB/phpbb/captcha/plugins/qa.php b/phpBB/phpbb/captcha/plugins/qa.php index ffc67a7706..3e48df00b2 100644 --- a/phpBB/phpbb/captcha/plugins/qa.php +++ b/phpBB/phpbb/captcha/plugins/qa.php @@ -323,14 +323,6 @@ class qa $db->sql_freeresult($result); } - /** - * API function - we don't drop the tables here, as that would cause the loss of all entered questions. - */ - function uninstall() - { - $this->garbage_collect(0); - } - /** * API function - see what has to be done to validate */ diff --git a/phpBB/phpbb/captcha/plugins/recaptcha.php b/phpBB/phpbb/captcha/plugins/recaptcha.php index a5dcf4efc6..1c080ba67b 100644 --- a/phpBB/phpbb/captcha/plugins/recaptcha.php +++ b/phpBB/phpbb/captcha/plugins/recaptcha.php @@ -179,11 +179,6 @@ class recaptcha extends captcha_abstract return $hidden_fields; } - function uninstall() - { - $this->garbage_collect(0); - } - function validate() { if (!parent::validate()) From edce13c7779cead54d94b0fac26e96f5dc6eaca0 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Sat, 5 Oct 2024 17:11:30 +0200 Subject: [PATCH 09/38] [ticket/17414] Add get_hidden_fields to interface PHPBB-17414 --- phpBB/phpbb/captcha/plugins/legacy_wrapper.php | 13 +++++++++++++ phpBB/phpbb/captcha/plugins/plugin_interface.php | 7 +++++++ 2 files changed, 20 insertions(+) diff --git a/phpBB/phpbb/captcha/plugins/legacy_wrapper.php b/phpBB/phpbb/captcha/plugins/legacy_wrapper.php index e57abbb967..5f2c701e07 100644 --- a/phpBB/phpbb/captcha/plugins/legacy_wrapper.php +++ b/phpBB/phpbb/captcha/plugins/legacy_wrapper.php @@ -74,6 +74,19 @@ class legacy_wrapper implements plugin_interface } } + /** + * {@inheritDoc} + */ + public function get_hidden_fields(): array + { + if (method_exists($this->legacy_captcha, 'get_hidden_fields')) + { + return $this->legacy_captcha->get_hidden_fields(); + } + + return []; + } + /** * {@inheritDoc} */ diff --git a/phpBB/phpbb/captcha/plugins/plugin_interface.php b/phpBB/phpbb/captcha/plugins/plugin_interface.php index 41e825fad3..9286c4867e 100644 --- a/phpBB/phpbb/captcha/plugins/plugin_interface.php +++ b/phpBB/phpbb/captcha/plugins/plugin_interface.php @@ -52,6 +52,13 @@ interface plugin_interface */ public function init(int $type): void; + /** + * Get hidden form fields for this captcha plugin + * + * @return array Hidden form fields + */ + public function get_hidden_fields(): array; + /** * Validate the captcha with the given request data * From c27c1fa7a3ddf35c4c79da43239804b83f6ca9e4 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Sat, 5 Oct 2024 20:56:35 +0200 Subject: [PATCH 10/38] [ticket/17414] Support adding onchange to select PHPBB-17414 --- phpBB/phpbb/template/twig/extension/forms.php | 1 + phpBB/styles/all/template/macros/forms/select.twig | 1 + 2 files changed, 2 insertions(+) diff --git a/phpBB/phpbb/template/twig/extension/forms.php b/phpBB/phpbb/template/twig/extension/forms.php index 5a3d9420db..b146aacfb9 100644 --- a/phpBB/phpbb/template/twig/extension/forms.php +++ b/phpBB/phpbb/template/twig/extension/forms.php @@ -183,6 +183,7 @@ class forms extends AbstractExtension 'GROUP_ONLY' => (bool) ($form_data['group_only'] ?? false), 'SIZE' => (int) ($form_data['size'] ?? 0), 'MULTIPLE' => (bool) ($form_data['multiple'] ?? false), + 'ONCHANGE' => (string) ($form_data['onchange'] ?? ''), ]); } catch (\Twig\Error\Error $e) diff --git a/phpBB/styles/all/template/macros/forms/select.twig b/phpBB/styles/all/template/macros/forms/select.twig index 827ccb1121..1ada063e57 100644 --- a/phpBB/styles/all/template/macros/forms/select.twig +++ b/phpBB/styles/all/template/macros/forms/select.twig @@ -8,6 +8,7 @@ name="{{ NAME }}" {% if TOGGLEABLE %}data-togglable-settings="true"{% endif %} {% if MULTIPLE %}multiple="multiple"{% endif %} + {% if ONCHANGE %}onchange="{{ ONCHANGE }}"{% endif %} {% if SIZE %}size="{{ SIZE }}"{% endif %}> {% for element in OPTIONS %} {% if not GROUP_ONLY and element.options %} From 8de8878d5e48b1c9febb311859bf3a10f9559ff3 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Sat, 5 Oct 2024 20:56:58 +0200 Subject: [PATCH 11/38] [ticket/17414] Move captcha acp html to html files PHPBB-17414 --- phpBB/adm/style/acp_captcha.html | 16 +++++++-------- phpBB/includes/acp/acp_captcha.php | 33 ++++++++++++++++++++---------- 2 files changed, 30 insertions(+), 19 deletions(-) diff --git a/phpBB/adm/style/acp_captcha.html b/phpBB/adm/style/acp_captcha.html index 4353becd2f..880adcbe97 100644 --- a/phpBB/adm/style/acp_captcha.html +++ b/phpBB/adm/style/acp_captcha.html @@ -8,12 +8,12 @@

{L_ACP_VC_EXT_GET_MORE}

- +{% if ERRORS %}
-

{L_WARNING}

-

{ERROR_MSG}

+

{{ lang('WARNING') }}

+

{{ ERRORS|join('
') }}

- +{% endif %}
@@ -47,10 +47,10 @@
{L_AVAILABLE_CAPTCHAS} -
-

{L_CAPTCHA_SELECT_EXPLAIN}
-
-
+
+

{{ lang('CAPTCHA_SELECT_EXPLAIN') }}
+
{{ FormsSelect(CAPTCHA_SELECT | merge({id: 'captcha_select', onchange: "(document.getElementById('acp_captcha')).submit()"})) }}
+

{L_CAPTCHA_CONFIGURE_EXPLAIN}
diff --git a/phpBB/includes/acp/acp_captcha.php b/phpBB/includes/acp/acp_captcha.php index 69e13463ab..e03d2a3901 100644 --- a/phpBB/includes/acp/acp_captcha.php +++ b/phpBB/includes/acp/acp_captcha.php @@ -95,7 +95,7 @@ class acp_captcha add_form_key($form_key); $submit = $request->variable('main_submit', false); - $error = $cfg_array = array(); + $errors = $cfg_array = array(); if ($submit) { @@ -103,13 +103,13 @@ class acp_captcha { $cfg_array[$config_var] = $request->variable($config_var, $options['default']); } - validate_config_vars($config_vars, $cfg_array, $error); + validate_config_vars($config_vars, $cfg_array, $errors); if (!check_form_key($form_key)) { - $error[] = $user->lang['FORM_INVALID']; + $errors[] = $user->lang['FORM_INVALID']; } - if ($error) + if ($errors) { $submit = false; } @@ -143,17 +143,24 @@ class acp_captcha } else { - $captcha_select = ''; + $captcha_options = []; foreach ($captchas['available'] as $value => $title) { - $current = ($selected !== false && $value == $selected) ? ' selected="selected"' : ''; - $captcha_select .= ''; + $captcha_options[] = [ + 'value' => $value, + 'label' => $user->lang($title), + 'selected' => $selected !== false && $value == $selected, + ]; } foreach ($captchas['unavailable'] as $value => $title) { - $current = ($selected !== false && $value == $selected) ? ' selected="selected"' : ''; - $captcha_select .= ''; + $captcha_options[] = [ + 'value' => $value, + 'label' => $user->lang($title), + 'selected' => $selected !== false && $value == $selected, + 'class' => 'disabled-option', + ]; } $demo_captcha = $factory->get_instance($selected); @@ -166,8 +173,12 @@ class acp_captcha $template->assign_vars(array( 'CAPTCHA_PREVIEW_TPL' => $demo_captcha->get_demo_template($id), 'S_CAPTCHA_HAS_CONFIG' => $demo_captcha->has_config(), - 'CAPTCHA_SELECT' => $captcha_select, - 'ERROR_MSG' => implode('
', $error), + 'CAPTCHA_SELECT' => [ + 'tag' => 'select', + 'name' => 'select_captcha', + 'options' => $captcha_options, + ], + 'ERRORS' => $errors, 'U_ACTION' => $this->u_action, )); From c02f8688c4964749b85bc5dcb9cfebc16faf5c35 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Sat, 5 Oct 2024 21:20:39 +0200 Subject: [PATCH 12/38] [ticket/17414] Convert acp captcha html to twig syntax PHPBB-17414 --- phpBB/adm/style/acp_captcha.html | 76 +++++++++++++++++--------------- 1 file changed, 41 insertions(+), 35 deletions(-) diff --git a/phpBB/adm/style/acp_captcha.html b/phpBB/adm/style/acp_captcha.html index 880adcbe97..0c9d3bf0af 100644 --- a/phpBB/adm/style/acp_captcha.html +++ b/phpBB/adm/style/acp_captcha.html @@ -1,12 +1,12 @@ - +{% include 'overall_header.html' %} -

{L_ACP_VC_SETTINGS}

+

{{ lang('ACP_VC_SETTINGS') }}

-

{L_ACP_VC_SETTINGS_EXPLAIN}

+

{{ lang('ACP_VC_SETTINGS_EXPLAIN') }}

-

{L_ACP_VC_EXT_GET_MORE}

+

{{ lang('ACP_VC_EXT_GET_MORE') }}

{% if ERRORS %}
@@ -15,65 +15,71 @@
{% endif %} - +
-{L_GENERAL_OPTIONS} +{{ lang('GENERAL_OPTIONS') }}
-

{L_VISUAL_CONFIRM_REG_EXPLAIN}
-
-
+

{{ lang('VISUAL_CONFIRM_REG_EXPLAIN') }}
+
+ + +
-

{L_REG_LIMIT_EXPLAIN}
-
+

{{ lang('REG_LIMIT_EXPLAIN') }}
+
-

{L_MAX_LOGIN_ATTEMPTS_EXPLAIN}
-
+

{{ lang('MAX_LOGIN_ATTEMPTS_EXPLAIN') }}
+
-

{L_VISUAL_CONFIRM_POST_EXPLAIN}
-
-
+

{{ lang('VISUAL_CONFIRM_POST_EXPLAIN') }}
+
+ + +
-

{L_VISUAL_CONFIRM_REFRESH_EXPLAIN}
-
-
+

{{ lang('VISUAL_CONFIRM_REFRESH_EXPLAIN') }}
+
+ + +
-{L_AVAILABLE_CAPTCHAS} +{{ lang('AVAILABLE_CAPTCHAS') }}

{{ lang('CAPTCHA_SELECT_EXPLAIN') }}
{{ FormsSelect(CAPTCHA_SELECT | merge({id: 'captcha_select', onchange: "(document.getElementById('acp_captcha')).submit()"})) }}
- -
-

{L_CAPTCHA_CONFIGURE_EXPLAIN}
-
-
- + {% if S_CAPTCHA_HAS_CONFIG %} +
+

{{ lang('CAPTCHA_CONFIGURE_EXPLAIN') }}
+
+
+ {% endif %}
- +{% if CAPTCHA_PREVIEW_TPL %}
- {L_PREVIEW} - + {{ lang('PREVIEW') }} + {% include CAPTCHA_PREVIEW_TPL %}
- +{% endif %}
- {L_ACP_SUBMIT_CHANGES} + {{ lang('ACP_SUBMIT_CHANGES') }}

-   -   +   +  

- {S_FORM_TOKEN} + {{ S_FORM_TOKEN }}
- +{% include 'overall_footer.html' %} From d85267707c01d69338014fb4cae5b12a66fc991d Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Sat, 5 Oct 2024 21:40:08 +0200 Subject: [PATCH 13/38] [ticket/17414] Add placeholder when there are no questions yet PHPBB-17414 --- phpBB/adm/style/captcha_qa_acp.html | 6 ++++++ phpBB/language/en/captcha_qa.php | 1 + 2 files changed, 7 insertions(+) diff --git a/phpBB/adm/style/captcha_qa_acp.html b/phpBB/adm/style/captcha_qa_acp.html index 26598c7c2b..1118a1f32e 100644 --- a/phpBB/adm/style/captcha_qa_acp.html +++ b/phpBB/adm/style/captcha_qa_acp.html @@ -18,11 +18,13 @@ {L_QUESTIONS} + {% if questions %} {L_QUESTION_TEXT} {L_QUESTION_LANG} {L_ACTION} + {% endif %} @@ -33,6 +35,10 @@ {{ question.QUESTION_LANG }} {{ ICON_EDIT }} {{ ICON_DELETE }} + {% else %} + + {{ lang('QA_NO_QUESTIONS') }} + {% endfor %} diff --git a/phpBB/language/en/captcha_qa.php b/phpBB/language/en/captcha_qa.php index 637c4e035e..b883166159 100644 --- a/phpBB/language/en/captcha_qa.php +++ b/phpBB/language/en/captcha_qa.php @@ -61,4 +61,5 @@ $lang = array_merge($lang, array( 'QA_ERROR_MSG' => 'Please fill in all fields and enter at least one answer.', 'QA_LAST_QUESTION' => 'You cannot delete all questions while the plugin is active.', + 'QA_NO_QUESTIONS' => 'There are no questions yet.', )); From 6a34de543b02eba8dde831bf96dfd3d780d72e9a Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Fri, 11 Oct 2024 21:16:07 +0200 Subject: [PATCH 14/38] [ticket/17414] Add get_template function to plugin interface PHPBB-17414 --- phpBB/phpbb/captcha/plugins/legacy_wrapper.php | 13 +++++++++++++ phpBB/phpbb/captcha/plugins/plugin_interface.php | 9 ++++++++- 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/phpBB/phpbb/captcha/plugins/legacy_wrapper.php b/phpBB/phpbb/captcha/plugins/legacy_wrapper.php index 5f2c701e07..9ce4973827 100644 --- a/phpBB/phpbb/captcha/plugins/legacy_wrapper.php +++ b/phpBB/phpbb/captcha/plugins/legacy_wrapper.php @@ -145,6 +145,19 @@ class legacy_wrapper implements plugin_interface return PHP_INT_MAX; } + /** + * {@inheritDoc} + */ + public function get_template(): string + { + if (method_exists($this->legacy_captcha, 'get_template')) + { + return $this->legacy_captcha->get_template(); + } + + return ''; + } + /** * {@inheritDoc} */ diff --git a/phpBB/phpbb/captcha/plugins/plugin_interface.php b/phpBB/phpbb/captcha/plugins/plugin_interface.php index 9286c4867e..0f668f207e 100644 --- a/phpBB/phpbb/captcha/plugins/plugin_interface.php +++ b/phpBB/phpbb/captcha/plugins/plugin_interface.php @@ -89,7 +89,14 @@ interface plugin_interface public function get_attempt_count(): int; /** - * Get template data for demo + * Get template filename for captcha + * + * @return string Template file name + */ + public function get_template(): string; + + /** + * Get template filename for demo * * @return string Demo template file name */ From 2a6609013da1841ac37cae3e6ccf56f642bb307a Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Fri, 11 Oct 2024 21:28:30 +0200 Subject: [PATCH 15/38] [ticket/17414] Remove request data from validate call PHPBB-17414 --- phpBB/includes/ucp/ucp_register.php | 2 +- phpBB/phpbb/auth/provider/db.php | 2 +- phpBB/phpbb/captcha/plugins/legacy_wrapper.php | 4 ++-- phpBB/phpbb/captcha/plugins/plugin_interface.php | 3 +-- 4 files changed, 5 insertions(+), 6 deletions(-) diff --git a/phpBB/includes/ucp/ucp_register.php b/phpBB/includes/ucp/ucp_register.php index 8ad37047cb..3a53148b93 100644 --- a/phpBB/includes/ucp/ucp_register.php +++ b/phpBB/includes/ucp/ucp_register.php @@ -293,7 +293,7 @@ class ucp_register if ($config['enable_confirm']) { - $vc_response = $captcha->validate($data); + $vc_response = $captcha->validate(); if ($vc_response !== false) { $error[] = $vc_response; diff --git a/phpBB/phpbb/auth/provider/db.php b/phpBB/phpbb/auth/provider/db.php index ae9d968076..395a48a9a5 100644 --- a/phpBB/phpbb/auth/provider/db.php +++ b/phpBB/phpbb/auth/provider/db.php @@ -177,7 +177,7 @@ class db extends base if ($show_captcha) { $captcha->init(CONFIRM_LOGIN); - $vc_response = $captcha->validate($row); + $vc_response = $captcha->validate(); if ($vc_response) { return array( diff --git a/phpBB/phpbb/captcha/plugins/legacy_wrapper.php b/phpBB/phpbb/captcha/plugins/legacy_wrapper.php index 9ce4973827..cc35920c70 100644 --- a/phpBB/phpbb/captcha/plugins/legacy_wrapper.php +++ b/phpBB/phpbb/captcha/plugins/legacy_wrapper.php @@ -90,11 +90,11 @@ class legacy_wrapper implements plugin_interface /** * {@inheritDoc} */ - public function validate(array $request_data): bool + public function validate(): bool { if (method_exists($this->legacy_captcha, 'validate')) { - $error = $this->legacy_captcha->validate($request_data); + $error = $this->legacy_captcha->validate(); if ($error) { $this->last_error = $error; diff --git a/phpBB/phpbb/captcha/plugins/plugin_interface.php b/phpBB/phpbb/captcha/plugins/plugin_interface.php index 0f668f207e..dc522ecd7e 100644 --- a/phpBB/phpbb/captcha/plugins/plugin_interface.php +++ b/phpBB/phpbb/captcha/plugins/plugin_interface.php @@ -62,10 +62,9 @@ interface plugin_interface /** * Validate the captcha with the given request data * - * @param array $request_data Request data for the captcha * @return bool True if request data was valid captcha reply, false if not */ - public function validate(array $request_data): bool; + public function validate(): bool; /** * Return whether captcha was solved From 52acd2709ba4c8fffaf29a7f42b4f182ce971faa Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Sat, 12 Oct 2024 10:48:37 +0200 Subject: [PATCH 16/38] [ticket/17413] Continue implementation of turnstile captcha PHPBB-17413 --- .../default/container/services_captcha.yml | 11 +++ phpBB/phpbb/captcha/plugins/turnstile.php | 82 ++++++++++++++++--- 2 files changed, 81 insertions(+), 12 deletions(-) diff --git a/phpBB/config/default/container/services_captcha.yml b/phpBB/config/default/container/services_captcha.yml index 8e9d829b47..98441f99f8 100644 --- a/phpBB/config/default/container/services_captcha.yml +++ b/phpBB/config/default/container/services_captcha.yml @@ -54,3 +54,14 @@ services: - ['set_name', ['core.captcha.plugins.recaptcha_v3']] tags: - { name: captcha.plugins } + + core.captcha.plugins.turnstile: + class: phpbb\captcha\plugins\turnstile + shared: false + arguments: + - '@config' + - '@language' + calls: + - ['set_name', ['core.captcha.plugins.turnstile']] + tags: + - { name: catpcha.plugins } diff --git a/phpBB/phpbb/captcha/plugins/turnstile.php b/phpBB/phpbb/captcha/plugins/turnstile.php index 130949e708..3a5e9231eb 100644 --- a/phpBB/phpbb/captcha/plugins/turnstile.php +++ b/phpBB/phpbb/captcha/plugins/turnstile.php @@ -13,32 +13,59 @@ namespace phpbb\captcha\plugins; -class turnstile extends captcha_abstract +use phpbb\config\config; +use phpbb\language\language; + +class turnstile implements plugin_interface { - /** @var \phpbb\config\config */ + private const API_ENDPOINT = 'https://api.cloudflare.com/client/v4/captcha/validate'; + + /** @var config */ protected $config; - public function __construct(\phpbb\config\config $config) + /** @var language */ + protected $language; + + public function __construct(config $config, language $language) { $this->config = $config; + $this->language = $language; } - public function is_available() + public function is_available(): bool { return ($this->config->offsetGet('captcha_turnstile_key') ?? false); } - public function get_template() + public function has_config(): bool { - return 'custom_captcha.html'; // Template file for displaying the CAPTCHA + return true; } - public function execute() + public function get_name(): string { - // Perform any necessary initialization or setup + return 'CAPTCHA_TURNSTILE'; } - public function validate() + public function init(int $type): void + { + $this->language->add_lang('captcha_turnstile'); + } + + public function get_hidden_fields(): array + { + $hidden_fields = []; + + // Required for posting page to store solved state + if ($this->solved) + { + $hidden_fields['confirm_code'] = $this->code; + } + $hidden_fields['confirm_id'] = $this->confirm_id; + return $hidden_fields; + } + + public function validate(): bool { // Implement server-side validation logic here // Example: Validate the submitted CAPTCHA value using Cloudflare API @@ -97,8 +124,39 @@ class turnstile extends captcha_abstract } } - public function get_generator_class() + public function is_solved(): bool { - throw new \Exception('No generator class given.'); + return false; } -} \ No newline at end of file + + public function reset(): void + { + // TODO: Implement reset() method. + } + + public function get_attempt_count(): int + { + // TODO: Implement get_attempt_count() method. + return 0; + } + + public function get_template(): string + { + return 'custom_captcha.html'; // Template file for displaying the CAPTCHA + } + + public function get_demo_template(): string + { + return ''; + } + + public function garbage_collect(int $confirm_type = 0): void + { + // TODO: Implement garbage_collect() method. + } + + public function acp_page($id, $module): void + { + // TODO: Implement acp_page() method. + } +} From b55b42d09f3913078c793d78efb9b15eed9aeb67 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Sat, 12 Oct 2024 20:36:12 +0200 Subject: [PATCH 17/38] [ticket/17413] Add migration for turnstile and acp demo PHPBB-17413 --- .../adm/style/captcha_turnstile_acp_demo.html | 23 +++++++++ .../default/container/services_captcha.yml | 2 +- phpBB/language/en/captcha_turnstile.php | 41 ++++++++++++++++ .../phpbb/captcha/plugins/legacy_wrapper.php | 11 +++++ .../captcha/plugins/plugin_interface.php | 7 +++ phpBB/phpbb/captcha/plugins/turnstile.php | 17 ++++++- .../migration/data/v400/turnstile_captcha.php | 48 +++++++++++++++++++ 7 files changed, 146 insertions(+), 3 deletions(-) create mode 100644 phpBB/adm/style/captcha_turnstile_acp_demo.html create mode 100644 phpBB/language/en/captcha_turnstile.php create mode 100644 phpBB/phpbb/db/migration/data/v400/turnstile_captcha.php diff --git a/phpBB/adm/style/captcha_turnstile_acp_demo.html b/phpBB/adm/style/captcha_turnstile_acp_demo.html new file mode 100644 index 0000000000..5d21532783 --- /dev/null +++ b/phpBB/adm/style/captcha_turnstile_acp_demo.html @@ -0,0 +1,23 @@ +
+
+
+{% INCLUDEJS 'https://challenges.cloudflare.com/turnstile/v0/api.js' %} + diff --git a/phpBB/config/default/container/services_captcha.yml b/phpBB/config/default/container/services_captcha.yml index 98441f99f8..b07f842ca8 100644 --- a/phpBB/config/default/container/services_captcha.yml +++ b/phpBB/config/default/container/services_captcha.yml @@ -64,4 +64,4 @@ services: calls: - ['set_name', ['core.captcha.plugins.turnstile']] tags: - - { name: catpcha.plugins } + - { name: captcha.plugins } diff --git a/phpBB/language/en/captcha_turnstile.php b/phpBB/language/en/captcha_turnstile.php new file mode 100644 index 0000000000..c091b440c1 --- /dev/null +++ b/phpBB/language/en/captcha_turnstile.php @@ -0,0 +1,41 @@ + +* @license GNU General Public License, version 2 (GPL-2.0) +* +* For full copyright and license information, please see +* the docs/CREDITS.txt file. +* +*/ + +/** +* DO NOT CHANGE +*/ +if (!defined('IN_PHPBB')) +{ + exit; +} + +if (empty($lang) || !is_array($lang)) +{ + $lang = []; +} + +// DEVELOPERS PLEASE NOTE +// +// All language files should use UTF-8 as their encoding and the files must not contain a BOM. +// +// Placeholders can now contain order information, e.g. instead of +// 'Page %s of %s' you can (and should) write 'Page %1$s of %2$s', this allows +// translators to re-order the output of data while ensuring it remains correct +// +// You do not need this where single placeholders are used, e.g. 'Message %d' is fine +// equally where a string contains only two placeholders which are used to wrap text +// in a url you again do not need to specify an order e.g., 'Click %sHERE%s' is fine + +$lang = array_merge($lang, [ + 'CAPTCHA_TURNSTILE' => 'Turnstile', +]); diff --git a/phpBB/phpbb/captcha/plugins/legacy_wrapper.php b/phpBB/phpbb/captcha/plugins/legacy_wrapper.php index cc35920c70..bc317e711a 100644 --- a/phpBB/phpbb/captcha/plugins/legacy_wrapper.php +++ b/phpBB/phpbb/captcha/plugins/legacy_wrapper.php @@ -63,6 +63,17 @@ class legacy_wrapper implements plugin_interface return false; } + /** + * {@inheritDoc} + */ + public function set_name(string $name): void + { + if (method_exists($this->legacy_captcha, 'set_name')) + { + $this->legacy_captcha->set_name($name); + } + } + /** * {@inheritDoc} */ diff --git a/phpBB/phpbb/captcha/plugins/plugin_interface.php b/phpBB/phpbb/captcha/plugins/plugin_interface.php index dc522ecd7e..f52643544e 100644 --- a/phpBB/phpbb/captcha/plugins/plugin_interface.php +++ b/phpBB/phpbb/captcha/plugins/plugin_interface.php @@ -44,6 +44,13 @@ interface plugin_interface */ public function get_name(): string; + /** + * Set the service name of the plugin + * + * @param string $name + */ + public function set_name(string $name): void; + /** * Display the captcha for the specified type * diff --git a/phpBB/phpbb/captcha/plugins/turnstile.php b/phpBB/phpbb/captcha/plugins/turnstile.php index 3a5e9231eb..2aa4ca3d0c 100644 --- a/phpBB/phpbb/captcha/plugins/turnstile.php +++ b/phpBB/phpbb/captcha/plugins/turnstile.php @@ -26,6 +26,8 @@ class turnstile implements plugin_interface /** @var language */ protected $language; + protected string $service_name = ''; + public function __construct(config $config, language $language) { $this->config = $config; @@ -34,7 +36,10 @@ class turnstile implements plugin_interface public function is_available(): bool { - return ($this->config->offsetGet('captcha_turnstile_key') ?? false); + $this->language->add_lang('captcha_turnstile'); + + return !empty($this->config->offsetGet('captcha_turnstile_sitekey')) + && !empty($this->config->offsetGet('captcha_turnstile_secret')); } public function has_config(): bool @@ -47,6 +52,14 @@ class turnstile implements plugin_interface return 'CAPTCHA_TURNSTILE'; } + /** + * {@inheritDoc} + */ + public function set_name(string $name): void + { + $this->service_name = $name; + } + public function init(int $type): void { $this->language->add_lang('captcha_turnstile'); @@ -147,7 +160,7 @@ class turnstile implements plugin_interface public function get_demo_template(): string { - return ''; + return 'captcha_turnstile_acp_demo.html'; } public function garbage_collect(int $confirm_type = 0): void diff --git a/phpBB/phpbb/db/migration/data/v400/turnstile_captcha.php b/phpBB/phpbb/db/migration/data/v400/turnstile_captcha.php new file mode 100644 index 0000000000..06111c9306 --- /dev/null +++ b/phpBB/phpbb/db/migration/data/v400/turnstile_captcha.php @@ -0,0 +1,48 @@ + + * @license GNU General Public License, version 2 (GPL-2.0) + * + * For full copyright and license information, please see + * the docs/CREDITS.txt file. + * + */ + +namespace phpbb\db\migration\data\v400; + +use phpbb\db\migration\migration; + +class turnstile_captcha extends migration +{ + public function effectively_installed(): bool + { + return $this->config->offsetExists('captcha_turnstile_sitekey') + && $this->config->offsetExists('captcha_turnstile_secret'); + } + + public static function depends_on(): array + { + return [ + '\phpbb\db\migration\data\v400\dev', + ]; + } + + public function update_data(): array + { + return [ + ['config.add', ['captcha_turnstile_sitekey', '']], + ['config.add', ['captcha_turnstile_secret', '']], + ]; + } + + public function revert_data(): array + { + return [ + ['config.remove', ['captcha_turnstile_sitekey']], + ['config.remove', ['captcha_turnstile_secret']], + ]; + } +} From 2500f722abe074a1905ee1346b26a971f4d10b6f Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Sat, 12 Oct 2024 21:08:50 +0200 Subject: [PATCH 18/38] [ticket/17413] Add acp settings for turnstile captcha PHPBB-17413 --- phpBB/adm/style/captcha_turnstile_acp.html | 50 +++++++++++ .../default/container/services_captcha.yml | 4 + phpBB/language/en/captcha_turnstile.php | 6 +- .../captcha/plugins/plugin_interface.php | 7 ++ phpBB/phpbb/captcha/plugins/turnstile.php | 86 ++++++++++++++++++- 5 files changed, 148 insertions(+), 5 deletions(-) create mode 100644 phpBB/adm/style/captcha_turnstile_acp.html diff --git a/phpBB/adm/style/captcha_turnstile_acp.html b/phpBB/adm/style/captcha_turnstile_acp.html new file mode 100644 index 0000000000..446a0f436b --- /dev/null +++ b/phpBB/adm/style/captcha_turnstile_acp.html @@ -0,0 +1,50 @@ +{% include('overall_header.html') %} + + + +

{{ lang('ACP_VC_SETTINGS') }}

+ +

{{ lang('ACP_VC_SETTINGS_EXPLAIN') }}

+ +
+
+ {{ lang('GENERAL_OPTIONS') }} +
+
+
+ {{ lang('CAPTCHA_TURNSTILE_SITEKEY_EXPLAIN') }} +
+
+
+
+
+
+ {{ lang('CAPTCHA_TURNSTILE_SECRET_EXPLAIN') }} +
+
+
+
+
+ {{ lang('PREVIEW') }} + {% if PREVIEW %} +
+

{{ lang('WARNING') }}

+

{{ lang('CAPTCHA_PREVIEW_MSG') }}

+
+ {% endif %} + {% include(CAPTCHA_PREVIEW) %} +
+ +
+ {{ lang('ACP_SUBMIT_CHANGES') }} +

+   +   +

+ + + {{ S_FORM_TOKEN }} +
+
+ +{% include('overall_footer.html') %} diff --git a/phpBB/config/default/container/services_captcha.yml b/phpBB/config/default/container/services_captcha.yml index b07f842ca8..09742f2eb8 100644 --- a/phpBB/config/default/container/services_captcha.yml +++ b/phpBB/config/default/container/services_captcha.yml @@ -61,6 +61,10 @@ services: arguments: - '@config' - '@language' + - '@log' + - '@request' + - '@template' + - '@user' calls: - ['set_name', ['core.captcha.plugins.turnstile']] tags: diff --git a/phpBB/language/en/captcha_turnstile.php b/phpBB/language/en/captcha_turnstile.php index c091b440c1..0b60867dd1 100644 --- a/phpBB/language/en/captcha_turnstile.php +++ b/phpBB/language/en/captcha_turnstile.php @@ -37,5 +37,9 @@ if (empty($lang) || !is_array($lang)) // in a url you again do not need to specify an order e.g., 'Click %sHERE%s' is fine $lang = array_merge($lang, [ - 'CAPTCHA_TURNSTILE' => 'Turnstile', + 'CAPTCHA_TURNSTILE' => 'Turnstile', + 'CAPTCHA_TURNSTILE_SITEKEY' => 'Sitekey', + 'CAPTCHA_TURNSTILE_SITEKEY_EXPLAIN' => 'Your Turnstile sitekey. The sitekey can be retrieved from your Cloudflare dashboard.', + 'CAPTCHA_TURNSTILE_SECRET' => 'Secret key', + 'CAPTCHA_TURNSTILE_SECRET_EXPLAIN' => 'Your Turnstile secret key. The secret key can be retrieved from your Cloudflare dashboard.', ]); diff --git a/phpBB/phpbb/captcha/plugins/plugin_interface.php b/phpBB/phpbb/captcha/plugins/plugin_interface.php index f52643544e..ee2ba8461d 100644 --- a/phpBB/phpbb/captcha/plugins/plugin_interface.php +++ b/phpBB/phpbb/captcha/plugins/plugin_interface.php @@ -116,5 +116,12 @@ interface plugin_interface */ public function garbage_collect(int $confirm_type = 0): void; + /** + * Display acp page + * + * @param mixed $id ACP module id + * @param mixed $module ACP module name + * @return void + */ public function acp_page($id, $module): void; } diff --git a/phpBB/phpbb/captcha/plugins/turnstile.php b/phpBB/phpbb/captcha/plugins/turnstile.php index 2aa4ca3d0c..0bb731d147 100644 --- a/phpBB/phpbb/captcha/plugins/turnstile.php +++ b/phpBB/phpbb/captcha/plugins/turnstile.php @@ -15,23 +15,54 @@ namespace phpbb\captcha\plugins; use phpbb\config\config; use phpbb\language\language; +use phpbb\log\log_interface; +use phpbb\request\request_interface; +use phpbb\template\template; +use phpbb\user; class turnstile implements plugin_interface { private const API_ENDPOINT = 'https://api.cloudflare.com/client/v4/captcha/validate'; /** @var config */ - protected $config; + protected config $config; /** @var language */ - protected $language; + protected language $language; + /** @var log_interface */ + protected log_interface $log; + + /** @var request_interface */ + protected request_interface $request; + + /** @var template */ + protected template $template; + + /** @var user */ + protected user $user; + + /** @var string Service name */ protected string $service_name = ''; - public function __construct(config $config, language $language) + /** + * Constructor for turnstile captcha plugin + * + * @param config $config + * @param language $language + * @param log_interface $log + * @param request_interface $request + * @param template $template + * @param user $user + */ + public function __construct(config $config, language $language, log_interface $log, request_interface $request, template $template, user $user) { $this->config = $config; $this->language = $language; + $this->log = $log; + $this->request = $request; + $this->template = $template; + $this->user = $user; } public function is_available(): bool @@ -168,8 +199,55 @@ class turnstile implements plugin_interface // TODO: Implement garbage_collect() method. } + /** + * {@inheritDoc} + */ public function acp_page($id, $module): void { - // TODO: Implement acp_page() method. + $captcha_vars = [ + 'captcha_turnstile_sitekey' => 'CAPTCHA_TURNSTILE_SITEKEY', + 'captcha_turnstile_secret' => 'CAPTCHA_TURNSTILE_SECRET', + ]; + + $module->tpl_name = 'captcha_turnstile_acp'; + $module->page_title = 'ACP_VC_SETTINGS'; + $form_key = 'acp_captcha'; + add_form_key($form_key); + + $submit = $this->request->is_set_post('submit'); + + if ($submit && check_form_key($form_key)) + { + $captcha_vars = array_keys($captcha_vars); + foreach ($captcha_vars as $captcha_var) + { + $value = $this->request->variable($captcha_var, ''); + if ($value) + { + $this->config->set($captcha_var, $value); + } + } + + $this->log->add('admin', $this->user->data['user_id'], $this->user->ip, 'LOG_CONFIG_VISUAL'); + trigger_error($this->language->lang('CONFIG_UPDATED') . adm_back_link($module->u_action)); + } + else if ($submit) + { + trigger_error($this->language->lang('FORM_INVALID') . adm_back_link($module->u_action)); + } + else + { + foreach ($captcha_vars as $captcha_var => $template_var) + { + $var = $this->request->is_set($captcha_var) ? $this->request->variable($captcha_var, '') : $this->config->offsetGet($captcha_var);; + $this->template->assign_var($template_var, $var); + } + + $this->template->assign_vars(array( + 'CAPTCHA_PREVIEW' => $this->get_demo_template(), + 'CAPTCHA_NAME' => $this->service_name, + 'U_ACTION' => $module->u_action, + )); + } } } From 01dd0b168a5829fe4f5c3cfff31828ca7fcceaf1 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Sun, 13 Oct 2024 08:53:11 +0200 Subject: [PATCH 19/38] [ticket/17414] Create abstract base class for captchas PHPBB-17414 --- .../default/container/services_captcha.yml | 1 + phpBB/phpbb/captcha/plugins/base.php | 62 +++++++++++++++++++ phpBB/phpbb/captcha/plugins/turnstile.php | 11 +++- 3 files changed, 71 insertions(+), 3 deletions(-) create mode 100644 phpBB/phpbb/captcha/plugins/base.php diff --git a/phpBB/config/default/container/services_captcha.yml b/phpBB/config/default/container/services_captcha.yml index 09742f2eb8..d4f1ad863a 100644 --- a/phpBB/config/default/container/services_captcha.yml +++ b/phpBB/config/default/container/services_captcha.yml @@ -60,6 +60,7 @@ services: shared: false arguments: - '@config' + - '@dbal.conn' - '@language' - '@log' - '@request' diff --git a/phpBB/phpbb/captcha/plugins/base.php b/phpBB/phpbb/captcha/plugins/base.php new file mode 100644 index 0000000000..2b07ccaa64 --- /dev/null +++ b/phpBB/phpbb/captcha/plugins/base.php @@ -0,0 +1,62 @@ +db = $db; + } + + /** + * @inheritDoc + */ + public function garbage_collect(int $confirm_type = 0): 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); + $result = $this->db->sql_query($sql); + + if ($row = $this->db->sql_fetchrow($result)) + { + $sql_in = []; + do + { + $sql_in[] = (string) $row['session_id']; + } + while ($row = $this->db->sql_fetchrow($result)); + + if (count($sql_in)) + { + $sql = 'DELETE FROM ' . CONFIRM_TABLE . ' + WHERE ' . $this->db->sql_in_set('session_id', $sql_in); + $this->db->sql_query($sql); + } + } + $this->db->sql_freeresult($result); + } +} diff --git a/phpBB/phpbb/captcha/plugins/turnstile.php b/phpBB/phpbb/captcha/plugins/turnstile.php index 0bb731d147..210ae03610 100644 --- a/phpBB/phpbb/captcha/plugins/turnstile.php +++ b/phpBB/phpbb/captcha/plugins/turnstile.php @@ -14,13 +14,15 @@ namespace phpbb\captcha\plugins; use phpbb\config\config; +use phpbb\db\driver\driver; +use phpbb\db\driver\driver_interface; use phpbb\language\language; use phpbb\log\log_interface; use phpbb\request\request_interface; use phpbb\template\template; use phpbb\user; -class turnstile implements plugin_interface +class turnstile extends base { private const API_ENDPOINT = 'https://api.cloudflare.com/client/v4/captcha/validate'; @@ -49,14 +51,17 @@ class turnstile implements plugin_interface * Constructor for turnstile captcha plugin * * @param config $config + * @param driver_interface $db * @param language $language * @param log_interface $log * @param request_interface $request * @param template $template * @param user $user */ - public function __construct(config $config, language $language, log_interface $log, request_interface $request, template $template, user $user) + public function __construct(config $config, driver_interface $db, language $language, log_interface $log, request_interface $request, template $template, user $user) { + parent::__construct($db); + $this->config = $config; $this->language = $language; $this->log = $log; @@ -103,7 +108,7 @@ class turnstile implements plugin_interface // Required for posting page to store solved state if ($this->solved) { - $hidden_fields['confirm_code'] = $this->code; + $hidden_fields['confirm_code'] = $this->confirm_code; } $hidden_fields['confirm_id'] = $this->confirm_id; return $hidden_fields; From 8290cdb7e734679f093f100a23d161e65eba0bfb Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Sun, 13 Oct 2024 11:04:59 +0200 Subject: [PATCH 20/38] [ticket/17413] Make turnstile captcha work on registration page PHPBB-17413 --- .../adm/style/captcha_turnstile_acp_demo.html | 2 +- phpBB/language/en/captcha_turnstile.php | 13 +- phpBB/phpbb/captcha/plugins/base.php | 19 +++ .../phpbb/captcha/plugins/legacy_wrapper.php | 8 ++ .../captcha/plugins/plugin_interface.php | 7 + phpBB/phpbb/captcha/plugins/turnstile.php | 123 ++++++++++-------- .../prosilver/template/captcha_turnstile.html | 12 ++ 7 files changed, 127 insertions(+), 57 deletions(-) create mode 100644 phpBB/styles/prosilver/template/captcha_turnstile.html diff --git a/phpBB/adm/style/captcha_turnstile_acp_demo.html b/phpBB/adm/style/captcha_turnstile_acp_demo.html index 5d21532783..348e96967a 100644 --- a/phpBB/adm/style/captcha_turnstile_acp_demo.html +++ b/phpBB/adm/style/captcha_turnstile_acp_demo.html @@ -1,7 +1,7 @@
-{% INCLUDEJS 'https://challenges.cloudflare.com/turnstile/v0/api.js' %} +{% INCLUDEJS U_TURNSTILE_SCRIPT %} + + {# The cf-turnstile class is used in JavaScript #} +
+{% else %} + {{ lang('CAPTCHA_TURNSTILE_NOT_AVAILABLE') }} +{% endif %} From c382f81222b856030561ca76879580c9c8c8dac8 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Sun, 13 Oct 2024 14:50:12 +0200 Subject: [PATCH 21/38] [ticket/17413] Add language and theme settings for turnstile PHPBB-17413 --- phpBB/adm/style/captcha_turnstile_acp.html | 16 ++++++++++++- .../adm/style/captcha_turnstile_acp_demo.html | 2 +- phpBB/install/schemas/schema_data.sql | 3 +++ phpBB/language/en/captcha_turnstile.php | 7 +++++- phpBB/language/en/composer.json | 3 ++- phpBB/phpbb/captcha/plugins/turnstile.php | 23 ++++++++++++++++--- .../migration/data/v400/turnstile_captcha.php | 5 +++- phpBB/phpbb/language/language.php | 1 + phpBB/phpbb/language/language_file_helper.php | 21 +++++++++-------- .../prosilver/template/captcha_turnstile.html | 2 +- 10 files changed, 64 insertions(+), 19 deletions(-) diff --git a/phpBB/adm/style/captcha_turnstile_acp.html b/phpBB/adm/style/captcha_turnstile_acp.html index 446a0f436b..a187e1e41b 100644 --- a/phpBB/adm/style/captcha_turnstile_acp.html +++ b/phpBB/adm/style/captcha_turnstile_acp.html @@ -23,10 +23,24 @@
+
+
+ +
{{ lang('CAPTCHA_TURNSTILE_THEME_EXPLAIN') }} +
+
+ {% for theme in CAPTCHA_TURNSTILE_THEMES %} + + {% endfor %} +
+
{{ lang('PREVIEW') }} - {% if PREVIEW %} + {% if PREVIEW %}

{{ lang('WARNING') }}

{{ lang('CAPTCHA_PREVIEW_MSG') }}

diff --git a/phpBB/adm/style/captcha_turnstile_acp_demo.html b/phpBB/adm/style/captcha_turnstile_acp_demo.html index 348e96967a..757d0a4bcb 100644 --- a/phpBB/adm/style/captcha_turnstile_acp_demo.html +++ b/phpBB/adm/style/captcha_turnstile_acp_demo.html @@ -1,5 +1,5 @@
-
+
{% INCLUDEJS U_TURNSTILE_SCRIPT %} {# The cf-turnstile class is used in JavaScript #} -
+
{% else %} {{ lang('CAPTCHA_TURNSTILE_NOT_AVAILABLE') }} {% endif %} From 589fe70be6298b719241570860a27dd802dfdcbf Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Sun, 13 Oct 2024 14:57:45 +0200 Subject: [PATCH 22/38] [ticket/17414] Change handling of validate to new logic PHPBB-17414 --- phpBB/includes/ucp/ucp_register.php | 5 ++--- phpBB/phpbb/auth/provider/db.php | 5 ++--- phpBB/phpbb/report/controller/report.php | 14 +++++++------- phpBB/posting.php | 10 ++-------- 4 files changed, 13 insertions(+), 21 deletions(-) diff --git a/phpBB/includes/ucp/ucp_register.php b/phpBB/includes/ucp/ucp_register.php index 3a53148b93..ad2ad79dbe 100644 --- a/phpBB/includes/ucp/ucp_register.php +++ b/phpBB/includes/ucp/ucp_register.php @@ -293,10 +293,9 @@ class ucp_register if ($config['enable_confirm']) { - $vc_response = $captcha->validate(); - if ($vc_response !== false) + if ($captcha->validate() !== true) { - $error[] = $vc_response; + $error[] = $captcha->get_error(); } if ($config['max_reg_attempts'] && $captcha->get_attempt_count() > $config['max_reg_attempts']) diff --git a/phpBB/phpbb/auth/provider/db.php b/phpBB/phpbb/auth/provider/db.php index 395a48a9a5..6911adfaff 100644 --- a/phpBB/phpbb/auth/provider/db.php +++ b/phpBB/phpbb/auth/provider/db.php @@ -176,9 +176,8 @@ class db extends base // Every auth module is able to define what to do by itself... if ($show_captcha) { - $captcha->init(CONFIRM_LOGIN); - $vc_response = $captcha->validate(); - if ($vc_response) + $captcha->init(\phpbb\captcha\plugins\plugin_interface::CONFIRM_LOGIN); + if ($captcha->validate() !== true) { return array( 'status' => LOGIN_ERROR_ATTEMPTS, diff --git a/phpBB/phpbb/report/controller/report.php b/phpBB/phpbb/report/controller/report.php index b8e2b04583..7d41c1c47f 100644 --- a/phpBB/phpbb/report/controller/report.php +++ b/phpBB/phpbb/report/controller/report.php @@ -13,6 +13,7 @@ namespace phpbb\report\controller; +use phpbb\captcha\plugins\plugin_interface; use phpbb\exception\http_exception; use phpbb\report\report_handler_interface; use Symfony\Component\HttpFoundation\RedirectResponse; @@ -131,7 +132,7 @@ class report if ($this->config['enable_post_confirm'] && !$this->user->data['is_registered']) { $captcha = $this->captcha_factory->get_instance($this->config['captcha_plugin']); - $captcha->init(CONFIRM_REPORT); + $captcha->init(plugin_interface::CONFIRM_REPORT); } //Has the report been cancelled? @@ -140,7 +141,7 @@ class report return new RedirectResponse($redirect_url, 302); } - // Check CAPTCHA, if the form was submited + // Check CAPTCHA, if the form was submitted if (!empty($submit) && isset($captcha)) { $captcha_template_array = $this->check_captcha($captcha); @@ -298,18 +299,17 @@ class report /** * Check CAPTCHA * - * @param object $captcha A phpBB CAPTCHA object + * @param plugin_interface $captcha A phpBB CAPTCHA object * @return array template variables which ensures that CAPTCHA's work correctly */ - protected function check_captcha($captcha) + protected function check_captcha(plugin_interface $captcha) { $error = array(); $captcha_hidden_fields = ''; - $visual_confirmation_response = $captcha->validate(); - if ($visual_confirmation_response) + if ($captcha->validate() !== true) { - $error[] = $visual_confirmation_response; + $error[] = $captcha->get_error(); } if (count($error) === 0) diff --git a/phpBB/posting.php b/phpBB/posting.php index 988b7f03e2..fb54f05ff6 100644 --- a/phpBB/posting.php +++ b/phpBB/posting.php @@ -1210,15 +1210,9 @@ if ($submit || $preview || $refresh) if ($config['enable_post_confirm'] && !$user->data['is_registered'] && in_array($mode, array('quote', 'post', 'reply'))) { - $captcha_data = array( - 'message' => $request->variable('message', '', true), - 'subject' => $request->variable('subject', '', true), - 'username' => $request->variable('username', '', true), - ); - $vc_response = $captcha->validate($captcha_data); - if ($vc_response) + if ($captcha->validate() !== true) { - $error[] = $vc_response; + $error[] = $captcha->get_error(); } } From 48454308aeee4bfd664eab2d4ecf8faad57a8f21 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Sun, 13 Oct 2024 17:13:01 +0200 Subject: [PATCH 23/38] [ticket/17414] Replace confirm types with enum PHPBB-17414 --- phpBB/includes/constants.php | 8 +++---- phpBB/includes/ucp/ucp_register.php | 2 +- phpBB/phpbb/auth/provider/db.php | 2 +- phpBB/phpbb/captcha/plugins/confirm_type.php | 24 +++++++++++++++++++ .../captcha/plugins/plugin_interface.php | 12 ++-------- phpBB/phpbb/captcha/plugins/turnstile.php | 2 +- phpBB/phpbb/report/controller/report.php | 3 ++- phpBB/posting.php | 2 +- 8 files changed, 36 insertions(+), 19 deletions(-) create mode 100644 phpBB/phpbb/captcha/plugins/confirm_type.php diff --git a/phpBB/includes/constants.php b/phpBB/includes/constants.php index 0ef409767d..825a328d39 100644 --- a/phpBB/includes/constants.php +++ b/phpBB/includes/constants.php @@ -152,13 +152,13 @@ define('FULL_FOLDER_DELETE', -2); define('FULL_FOLDER_HOLD', -1); // Confirm types -/** @deprecated 4.0.0-a1 Replaced by \phpbb\captcha\plugins\plugin_interface::CONFIRM_REGISTRATION, to be removed in 5.0.0-a1 */ +/** @deprecated 4.0.0-a1 Replaced by \phpbb\captcha\plugins\confirm_type::REGISTRATION, to be removed in 5.0.0-a1 */ define('CONFIRM_REG', 1); -/** @deprecated 4.0.0-a1 Replaced by \phpbb\captcha\plugins\plugin_interface::CONFIRM_LOGIN, to be removed in 5.0.0-a1 */ +/** @deprecated 4.0.0-a1 Replaced by \phpbb\captcha\plugins\confirm_type::LOGIN, to be removed in 5.0.0-a1 */ define('CONFIRM_LOGIN', 2); -/** @deprecated 4.0.0-a1 Replaced by \phpbb\captcha\plugins\plugin_interface::CONFIRM_POST, to be removed in 5.0.0-a1 */ +/** @deprecated 4.0.0-a1 Replaced by \phpbb\captcha\plugins\confirm_type::POST, to be removed in 5.0.0-a1 */ define('CONFIRM_POST', 3); -/** @deprecated 4.0.0-a1 Replaced by \phpbb\captcha\plugins\plugin_interface::CONFIRM_REPORT, to be removed in 5.0.0-a1 */ +/** @deprecated 4.0.0-a1 Replaced by \phpbb\captcha\plugins\confirm_type::REPORT, to be removed in 5.0.0-a1 */ define('CONFIRM_REPORT', 4); // Categories - Attachments diff --git a/phpBB/includes/ucp/ucp_register.php b/phpBB/includes/ucp/ucp_register.php index ad2ad79dbe..b8d0d78b40 100644 --- a/phpBB/includes/ucp/ucp_register.php +++ b/phpBB/includes/ucp/ucp_register.php @@ -238,7 +238,7 @@ class ucp_register /** @var \phpbb\captcha\factory $captcha_factory */ $captcha_factory = $phpbb_container->get('captcha.factory'); $captcha = $captcha_factory->get_instance($config['captcha_plugin']); - $captcha->init(\phpbb\captcha\plugins\plugin_interface::CONFIRM_REGISTRATION); + $captcha->init(\phpbb\captcha\plugins\confirm_type::REGISTRATION); } $timezone = $config['board_timezone']; diff --git a/phpBB/phpbb/auth/provider/db.php b/phpBB/phpbb/auth/provider/db.php index 6911adfaff..5f8ebd6ac3 100644 --- a/phpBB/phpbb/auth/provider/db.php +++ b/phpBB/phpbb/auth/provider/db.php @@ -176,7 +176,7 @@ class db extends base // Every auth module is able to define what to do by itself... if ($show_captcha) { - $captcha->init(\phpbb\captcha\plugins\plugin_interface::CONFIRM_LOGIN); + $captcha->init(\phpbb\captcha\plugins\confirm_type::LOGIN); if ($captcha->validate() !== true) { return array( diff --git a/phpBB/phpbb/captcha/plugins/confirm_type.php b/phpBB/phpbb/captcha/plugins/confirm_type.php new file mode 100644 index 0000000000..db8bb54bd2 --- /dev/null +++ b/phpBB/phpbb/captcha/plugins/confirm_type.php @@ -0,0 +1,24 @@ + + * @license GNU General Public License, version 2 (GPL-2.0) + * + * For full copyright and license information, please see + * the docs/CREDITS.txt file. + * + */ + +namespace phpbb\captcha\plugins; + +/** + * Confirmation types for CAPTCHA plugins + */ +enum confirm_type: int { + case REGISTRATION = 1; + case LOGIN = 2; + case POST = 3; + case REPORT = 4; +} diff --git a/phpBB/phpbb/captcha/plugins/plugin_interface.php b/phpBB/phpbb/captcha/plugins/plugin_interface.php index bc20b2e703..c6e4e6c6de 100644 --- a/phpBB/phpbb/captcha/plugins/plugin_interface.php +++ b/phpBB/phpbb/captcha/plugins/plugin_interface.php @@ -15,14 +15,6 @@ namespace phpbb\captcha\plugins; interface plugin_interface { - const CONFIRM_REGISTRATION = 1; - const CONFIRM_LOGIN = 2; - - const CONFIRM_POST = 3; - - const CONFIRM_REPORT = 4; - - /** * Check if the plugin is available * @@ -54,10 +46,10 @@ interface plugin_interface /** * Display the captcha for the specified type * - * @param int $type Type of captcha, should be one of the CONFIRMATION_* constants + * @param confirm_type $type Type of captcha, should be one of the CONFIRMATION_* constants * @return void */ - public function init(int $type): void; + public function init(confirm_type $type): void; /** * Get hidden form fields for this captcha plugin diff --git a/phpBB/phpbb/captcha/plugins/turnstile.php b/phpBB/phpbb/captcha/plugins/turnstile.php index b3152b2a23..f5bd22602b 100644 --- a/phpBB/phpbb/captcha/plugins/turnstile.php +++ b/phpBB/phpbb/captcha/plugins/turnstile.php @@ -121,7 +121,7 @@ class turnstile extends base /** * {@inheritDoc} */ - public function init(int $type): void + public function init(confirm_type $type): void { $this->language->add_lang('captcha_turnstile'); } diff --git a/phpBB/phpbb/report/controller/report.php b/phpBB/phpbb/report/controller/report.php index 7d41c1c47f..97dbdbd9bb 100644 --- a/phpBB/phpbb/report/controller/report.php +++ b/phpBB/phpbb/report/controller/report.php @@ -13,6 +13,7 @@ namespace phpbb\report\controller; +use phpbb\captcha\plugins\confirm_type; use phpbb\captcha\plugins\plugin_interface; use phpbb\exception\http_exception; use phpbb\report\report_handler_interface; @@ -132,7 +133,7 @@ class report if ($this->config['enable_post_confirm'] && !$this->user->data['is_registered']) { $captcha = $this->captcha_factory->get_instance($this->config['captcha_plugin']); - $captcha->init(plugin_interface::CONFIRM_REPORT); + $captcha->init(confirm_type::REPORT); } //Has the report been cancelled? diff --git a/phpBB/posting.php b/phpBB/posting.php index fb54f05ff6..9f8cce0ead 100644 --- a/phpBB/posting.php +++ b/phpBB/posting.php @@ -458,7 +458,7 @@ if ($config['enable_post_confirm'] && !$user->data['is_registered']) /** @var \phpbb\captcha\factory $captcha_factory */ $captcha_factory = $phpbb_container->get('captcha.factory'); $captcha = $captcha_factory->get_instance($config['captcha_plugin']); - $captcha->init(\phpbb\captcha\plugins\plugin_interface::CONFIRM_POST); + $captcha->init(\phpbb\captcha\plugins\confirm_type::POST); } // Is the user able to post within this forum? From cf16a76f0c710057cf2c30bcdccdc4f8fa00adea Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Sun, 13 Oct 2024 21:41:11 +0200 Subject: [PATCH 24/38] [ticket/17414] Move more functionality to base and fix turnstile language PHPBB-17414 --- phpBB/phpbb/captcha/plugins/base.php | 94 ++++++++++++++++++- phpBB/phpbb/captcha/plugins/confirm_type.php | 1 + .../phpbb/captcha/plugins/legacy_wrapper.php | 9 +- phpBB/phpbb/captcha/plugins/turnstile.php | 36 +------ .../prosilver/template/captcha_turnstile.html | 2 +- 5 files changed, 103 insertions(+), 39 deletions(-) diff --git a/phpBB/phpbb/captcha/plugins/base.php b/phpBB/phpbb/captcha/plugins/base.php index be3b77ade1..a22336b1e5 100644 --- a/phpBB/phpbb/captcha/plugins/base.php +++ b/phpBB/phpbb/captcha/plugins/base.php @@ -2,34 +2,118 @@ namespace phpbb\captcha\plugins; -use phpbb\captcha\plugins\plugin_interface; +use phpbb\config\config; use phpbb\db\driver\driver_interface; +use phpbb\request\request_interface; +use phpbb\user; abstract class base implements plugin_interface { + /** @var config */ + protected config $config; + /** @var driver_interface */ protected driver_interface $db; + /** @var request_interface */ + protected request_interface $request; + + /** @var user */ + protected user $user; + + /** @var int Attempts at solving the CAPTCHA */ + protected int $attempts = 0; + /** @var bool Resolved state of captcha */ protected bool $solved = false; - /** @var string Confirm code */ - protected string $confirm_code = ''; - /** @var string Confirm id hash */ protected string $confirm_id = ''; + /** @var confirm_type Confirmation type */ + protected confirm_type $type = confirm_type::UNDEFINED; + /** @var string Last error message */ protected string $last_error = ''; /** * Constructor for abstract captcha base class * + * @param config $config * @param driver_interface $db + * @param request_interface $request + * @param user $user */ - public function __construct(driver_interface $db) + public function __construct(config $config, driver_interface $db, request_interface $request, user $user) { + $this->config = $config; $this->db = $db; + $this->request = $request; + $this->user = $user; + } + + /** + * {@inheritDoc} + */ + public function init(confirm_type $type): void + { + $this->confirm_id = $this->request->variable('confirm_id', ''); + $this->type = $type; + + if (empty($this->confirm_id) || !$this->load_confirm_data()) + { + // we have no confirm ID, better get ready to display something + $this->generate_confirm_data(); + } + } + + /** + * Look up attempts from confirm table + */ + protected function load_confirm_data(): bool + { + $sql = 'SELECT attempts + FROM ' . CONFIRM_TABLE . " + WHERE confirm_id = '" . $this->db->sql_escape($this->confirm_id) . "' + AND session_id = '" . $this->db->sql_escape($this->user->session_id) . "' + AND confirm_type = " . (int) $this->type; + $result = $this->db->sql_query($sql); + $row = $this->db->sql_fetchrow($result); + $this->db->sql_freeresult($result); + + if ($row) + { + $this->attempts = $row['attempts']; + + return true; + } + + return false; + } + + /** + * Generate confirm data for tracking attempts + * + * @return void + */ + protected function generate_confirm_data(): void + { + $this->confirm_id = md5(unique_id()); + + $sql = 'INSERT INTO ' . CONFIRM_TABLE . ' ' . $this->db->sql_build_array('INSERT', array( + 'confirm_id' => $this->confirm_id, + 'session_id' => (string) $this->user->session_id, + 'confirm_type' => $this->type + )); + $this->db->sql_query($sql); + } + + /** + * {@inheritDoc} + */ + public function get_hidden_fields(): array + { + return ['confirm_id' => $this->confirm_id]; } /** diff --git a/phpBB/phpbb/captcha/plugins/confirm_type.php b/phpBB/phpbb/captcha/plugins/confirm_type.php index db8bb54bd2..e3e11edf77 100644 --- a/phpBB/phpbb/captcha/plugins/confirm_type.php +++ b/phpBB/phpbb/captcha/plugins/confirm_type.php @@ -17,6 +17,7 @@ namespace phpbb\captcha\plugins; * Confirmation types for CAPTCHA plugins */ enum confirm_type: int { + case UNDEFINED = 0; case REGISTRATION = 1; case LOGIN = 2; case POST = 3; diff --git a/phpBB/phpbb/captcha/plugins/legacy_wrapper.php b/phpBB/phpbb/captcha/plugins/legacy_wrapper.php index 48b91ef3d2..48575733cc 100644 --- a/phpBB/phpbb/captcha/plugins/legacy_wrapper.php +++ b/phpBB/phpbb/captcha/plugins/legacy_wrapper.php @@ -19,7 +19,12 @@ class legacy_wrapper implements plugin_interface private string $last_error; - public function __construct($legacy_captcha) + /** + * Constructor for legacy CAPTCHA wrapper + * + * @param object $legacy_captcha + */ + public function __construct(object $legacy_captcha) { $this->legacy_captcha = $legacy_captcha; } @@ -77,7 +82,7 @@ class legacy_wrapper implements plugin_interface /** * {@inheritDoc} */ - public function init(int $type): void + public function init(confirm_type $type): void { if (method_exists($this->legacy_captcha, 'init')) { diff --git a/phpBB/phpbb/captcha/plugins/turnstile.php b/phpBB/phpbb/captcha/plugins/turnstile.php index f5bd22602b..1a42c7adbf 100644 --- a/phpBB/phpbb/captcha/plugins/turnstile.php +++ b/phpBB/phpbb/captcha/plugins/turnstile.php @@ -16,7 +16,6 @@ namespace phpbb\captcha\plugins; use GuzzleHttp\Client; use GuzzleHttp\Exception\GuzzleException; use phpbb\config\config; -use phpbb\db\driver\driver; use phpbb\db\driver\driver_interface; use phpbb\language\language; use phpbb\log\log_interface; @@ -32,24 +31,15 @@ class turnstile extends base /** @var string API endpoint for turnstile verification */ private const VERIFY_ENDPOINT = 'https://challenges.cloudflare.com/turnstile/v0/siteverify'; - /** @var config */ - protected config $config; - /** @var language */ protected language $language; /** @var log_interface */ protected log_interface $log; - /** @var request_interface */ - protected request_interface $request; - /** @var template */ protected template $template; - /** @var user */ - protected user $user; - /** @var string Service name */ protected string $service_name = ''; @@ -73,14 +63,11 @@ class turnstile extends base */ public function __construct(config $config, driver_interface $db, language $language, log_interface $log, request_interface $request, template $template, user $user) { - parent::__construct($db); + parent::__construct($config, $db, $request, $user); - $this->config = $config; $this->language = $language; $this->log = $log; - $this->request = $request; $this->template = $template; - $this->user = $user; } /** @@ -88,7 +75,7 @@ class turnstile extends base */ public function is_available(): bool { - $this->init(0); + $this->init(confirm_type::UNDEFINED); return !empty($this->config->offsetGet('captcha_turnstile_sitekey')) && !empty($this->config->offsetGet('captcha_turnstile_secret')); @@ -126,22 +113,6 @@ class turnstile extends base $this->language->add_lang('captcha_turnstile'); } - /** - * {@inheritDoc} - */ - public function get_hidden_fields(): array - { - $hidden_fields = []; - - // Required for posting page to store solved state - if ($this->solved) - { - $hidden_fields['confirm_code'] = $this->confirm_code; - } - $hidden_fields['confirm_id'] = $this->confirm_id; - return $hidden_fields; - } - /** * {@inheritDoc} */ @@ -196,6 +167,9 @@ class turnstile extends base // TODO: Implement reset() method. } + /** + * {@inheritDoc} + */ public function get_attempt_count(): int { // TODO: Implement get_attempt_count() method. diff --git a/phpBB/styles/prosilver/template/captcha_turnstile.html b/phpBB/styles/prosilver/template/captcha_turnstile.html index f20d53e539..b8c8855502 100644 --- a/phpBB/styles/prosilver/template/captcha_turnstile.html +++ b/phpBB/styles/prosilver/template/captcha_turnstile.html @@ -6,7 +6,7 @@ {# The cf-turnstile class is used in JavaScript #} -
+
{% else %} {{ lang('CAPTCHA_TURNSTILE_NOT_AVAILABLE') }} {% endif %} From db25443bc56f20669e80911f7def4906b0683d76 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Tue, 15 Oct 2024 19:43:08 +0200 Subject: [PATCH 25/38] [ticket/17413] Clean up turnstile code and add attempt counting PHPBB-17413 --- phpBB/phpbb/captcha/plugins/base.php | 81 +++++++++++++++++-- phpBB/phpbb/captcha/plugins/turnstile.php | 52 ++++++------ .../prosilver/template/captcha_turnstile.html | 24 ++++-- 3 files changed, 120 insertions(+), 37 deletions(-) diff --git a/phpBB/phpbb/captcha/plugins/base.php b/phpBB/phpbb/captcha/plugins/base.php index a22336b1e5..d43fcec31e 100644 --- a/phpBB/phpbb/captcha/plugins/base.php +++ b/phpBB/phpbb/captcha/plugins/base.php @@ -4,6 +4,7 @@ namespace phpbb\captcha\plugins; use phpbb\config\config; use phpbb\db\driver\driver_interface; +use phpbb\language\language; use phpbb\request\request_interface; use phpbb\user; @@ -15,6 +16,9 @@ abstract class base implements plugin_interface /** @var driver_interface */ protected driver_interface $db; + /** @var language */ + protected language $language; + /** @var request_interface */ protected request_interface $request; @@ -24,9 +28,15 @@ abstract class base implements plugin_interface /** @var int Attempts at solving the CAPTCHA */ protected int $attempts = 0; + /** @var string Stored random CAPTCHA code */ + protected string $code = ''; + /** @var bool Resolved state of captcha */ protected bool $solved = false; + /** @var string User supplied confirm code */ + protected string $confirm_code = ''; + /** @var string Confirm id hash */ protected string $confirm_id = ''; @@ -41,13 +51,15 @@ abstract class base implements plugin_interface * * @param config $config * @param driver_interface $db + * @param language $language * @param request_interface $request * @param user $user */ - public function __construct(config $config, driver_interface $db, request_interface $request, user $user) + public function __construct(config $config, driver_interface $db, language $language, request_interface $request, user $user) { $this->config = $config; $this->db = $db; + $this->language = $language; $this->request = $request; $this->user = $user; } @@ -58,6 +70,7 @@ abstract class base implements plugin_interface public function init(confirm_type $type): void { $this->confirm_id = $this->request->variable('confirm_id', ''); + $this->confirm_code = $this->request->variable('confirm_code', ''); $this->type = $type; if (empty($this->confirm_id) || !$this->load_confirm_data()) @@ -67,16 +80,52 @@ abstract class base implements plugin_interface } } + /** + * {@inheritDoc} + */ + public function validate(): bool + { + if ($this->confirm_id && hash_equals($this->code, $this->confirm_code)) + { + return true; + } + + $this->increment_attempts(); + $this->last_error = $this->language->lang('CONFIRM_CODE_WRONG'); + return false; + } + + /** + * {@inheritDoc} + */ + public function reset(): void + { + $sql = 'DELETE FROM ' . CONFIRM_TABLE . " + WHERE session_id = '" . $this->db->sql_escape($this->user->session_id) . "' + AND confirm_type = " . $this->type->value; + $this->db->sql_query($sql); + + $this->generate_confirm_data(); + } + + /** + * {@inheritDoc} + */ + public function get_attempt_count(): int + { + return $this->attempts; + } + /** * Look up attempts from confirm table */ protected function load_confirm_data(): bool { - $sql = 'SELECT attempts + $sql = 'SELECT code, attempts FROM ' . CONFIRM_TABLE . " WHERE confirm_id = '" . $this->db->sql_escape($this->confirm_id) . "' AND session_id = '" . $this->db->sql_escape($this->user->session_id) . "' - AND confirm_type = " . (int) $this->type; + AND confirm_type = " . $this->type->value; $result = $this->db->sql_query($sql); $row = $this->db->sql_fetchrow($result); $this->db->sql_freeresult($result); @@ -84,6 +133,7 @@ abstract class base implements plugin_interface if ($row) { $this->attempts = $row['attempts']; + $this->code = $row['code']; return true; } @@ -98,22 +148,43 @@ abstract class base implements plugin_interface */ protected function generate_confirm_data(): void { + $this->code = gen_rand_string_friendly(CAPTCHA_MAX_CHARS); $this->confirm_id = md5(unique_id()); $sql = 'INSERT INTO ' . CONFIRM_TABLE . ' ' . $this->db->sql_build_array('INSERT', array( 'confirm_id' => $this->confirm_id, 'session_id' => (string) $this->user->session_id, - 'confirm_type' => $this->type + 'confirm_type' => $this->type->value, + 'code' => $this->code, )); $this->db->sql_query($sql); } + /** + * Increment number of attempts for confirm ID and session + * + * @return void + */ + protected function increment_attempts(): void + { + $sql = 'UPDATE ' . CONFIRM_TABLE . " + SET attempts = attempts + 1 + WHERE confirm_id = '{$this->db->sql_escape($this->confirm_id)}' + AND session_id = '{$this->db->sql_escape($this->user->session_id)}'"; + $this->db->sql_query($sql); + + $this->attempts++; + } + /** * {@inheritDoc} */ public function get_hidden_fields(): array { - return ['confirm_id' => $this->confirm_id]; + return [ + 'confirm_id' => $this->confirm_id, + 'confirm_code' => $this->solved === true ? $this->confirm_code : '', + ]; } /** diff --git a/phpBB/phpbb/captcha/plugins/turnstile.php b/phpBB/phpbb/captcha/plugins/turnstile.php index 1a42c7adbf..17e4154e8a 100644 --- a/phpBB/phpbb/captcha/plugins/turnstile.php +++ b/phpBB/phpbb/captcha/plugins/turnstile.php @@ -63,7 +63,7 @@ class turnstile extends base */ public function __construct(config $config, driver_interface $db, language $language, log_interface $log, request_interface $request, template $template, user $user) { - parent::__construct($config, $db, $request, $user); + parent::__construct($config, $db, $language, $request, $user); $this->language = $language; $this->log = $log; @@ -75,7 +75,7 @@ class turnstile extends base */ public function is_available(): bool { - $this->init(confirm_type::UNDEFINED); + $this->init($this->type); return !empty($this->config->offsetGet('captcha_turnstile_sitekey')) && !empty($this->config->offsetGet('captcha_turnstile_secret')); @@ -110,6 +110,8 @@ class turnstile extends base */ public function init(confirm_type $type): void { + parent::init($type); + $this->language->add_lang('captcha_turnstile'); } @@ -118,10 +120,22 @@ class turnstile extends base */ public function validate(): bool { + if (parent::validate()) + { + return true; + } + + $turnstile_response = $this->request->variable('cf-turnstile-response', ''); + if (!$turnstile_response) + { + // Return without checking against server without a turnstile response + return false; + } + // Retrieve form data for verification $form_data = [ 'secret' => $this->config['captcha_turnstile_secret'], - 'response' => $this->request->variable('cf-turnstile-response', ''), + 'response' => $turnstile_response, 'remoteip' => $this->request->header('CF-Connecting-IP'), //'idempotency_key' => $this->confirm_id, // check if we need this ]; @@ -150,6 +164,7 @@ class turnstile extends base if (isset($result['success']) && $result['success'] === true) { $this->solved = true; + $this->confirm_code = $this->code; return true; } else @@ -162,20 +177,6 @@ class turnstile extends base /** * {@inheritDoc} */ - public function reset(): void - { - // TODO: Implement reset() method. - } - - /** - * {@inheritDoc} - */ - public function get_attempt_count(): int - { - // TODO: Implement get_attempt_count() method. - return 0; - } - public function get_template(): string { if ($this->is_solved()) @@ -184,15 +185,19 @@ class turnstile extends base } $this->template->assign_vars([ - 'S_TURNSTILE_AVAILABLE' => $this->is_available(), - 'TURNSTILE_SITEKEY' => $this->config->offsetGet('captcha_turnstile_sitekey'), - 'TURNSTILE_THEME' => $this->config->offsetGet('captcha_turnstile_theme'), - 'U_TURNSTILE_SCRIPT' => self::SCRIPT_URL, + 'S_TURNSTILE_AVAILABLE' => $this->is_available(), + 'TURNSTILE_SITEKEY' => $this->config->offsetGet('captcha_turnstile_sitekey'), + 'TURNSTILE_THEME' => $this->config->offsetGet('captcha_turnstile_theme'), + 'U_TURNSTILE_SCRIPT' => self::SCRIPT_URL, + 'CONFIRM_TYPE_REGISTRATION' => (int) $this->type->value, ]); return 'captcha_turnstile.html'; } + /** + * {@inheritDoc} + */ public function get_demo_template(): string { $this->template->assign_vars([ @@ -203,11 +208,6 @@ class turnstile extends base return 'captcha_turnstile_acp_demo.html'; } - public function garbage_collect(int $confirm_type = 0): void - { - // TODO: Implement garbage_collect() method. - } - /** * {@inheritDoc} */ diff --git a/phpBB/styles/prosilver/template/captcha_turnstile.html b/phpBB/styles/prosilver/template/captcha_turnstile.html index b8c8855502..aed3f9299c 100644 --- a/phpBB/styles/prosilver/template/captcha_turnstile.html +++ b/phpBB/styles/prosilver/template/captcha_turnstile.html @@ -1,12 +1,24 @@ +{% if CONFIRM_TYPE_REGISTRATION %} +
+
+ +

{L_CONFIRMATION}

+
+{% endif %} {% if S_TURNSTILE_AVAILABLE %} - + - + - {# The cf-turnstile class is used in JavaScript #} -
+ {# The cf-turnstile class is used in JavaScript #} +
{% else %} {{ lang('CAPTCHA_TURNSTILE_NOT_AVAILABLE') }} {% endif %} +{% if CONFIRM_TYPE_REGISTRATION %} +
+
+
+{% endif %} From 9eb18f351bd41e3917f43ca09569043905fc2c6a Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Tue, 15 Oct 2024 20:09:21 +0200 Subject: [PATCH 26/38] [ticket/17413] Fix setting solved state on valid confirm code PHPBB-17413 --- phpBB/phpbb/captcha/plugins/base.php | 1 + phpBB/phpbb/captcha/plugins/turnstile.php | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/phpBB/phpbb/captcha/plugins/base.php b/phpBB/phpbb/captcha/plugins/base.php index d43fcec31e..25355bcb53 100644 --- a/phpBB/phpbb/captcha/plugins/base.php +++ b/phpBB/phpbb/captcha/plugins/base.php @@ -87,6 +87,7 @@ abstract class base implements plugin_interface { if ($this->confirm_id && hash_equals($this->code, $this->confirm_code)) { + $this->solved = true; return true; } diff --git a/phpBB/phpbb/captcha/plugins/turnstile.php b/phpBB/phpbb/captcha/plugins/turnstile.php index 17e4154e8a..70c33e6e21 100644 --- a/phpBB/phpbb/captcha/plugins/turnstile.php +++ b/phpBB/phpbb/captcha/plugins/turnstile.php @@ -189,7 +189,7 @@ class turnstile extends base 'TURNSTILE_SITEKEY' => $this->config->offsetGet('captcha_turnstile_sitekey'), 'TURNSTILE_THEME' => $this->config->offsetGet('captcha_turnstile_theme'), 'U_TURNSTILE_SCRIPT' => self::SCRIPT_URL, - 'CONFIRM_TYPE_REGISTRATION' => (int) $this->type->value, + 'CONFIRM_TYPE_REGISTRATION' => $this->type->value, ]); return 'captcha_turnstile.html'; From 61e265e4b4a35d749ac2677855ec51d68c4bc0a3 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Tue, 15 Oct 2024 21:05:01 +0200 Subject: [PATCH 27/38] [ticket/17413] Start implementation of unit tests PHPBB-17413 --- phpBB/phpbb/captcha/plugins/turnstile.php | 20 +- tests/captcha/turnstile_test.php | 254 ++++++++++++++++++++++ 2 files changed, 273 insertions(+), 1 deletion(-) create mode 100644 tests/captcha/turnstile_test.php diff --git a/phpBB/phpbb/captcha/plugins/turnstile.php b/phpBB/phpbb/captcha/plugins/turnstile.php index 70c33e6e21..bf9ada4831 100644 --- a/phpBB/phpbb/captcha/plugins/turnstile.php +++ b/phpBB/phpbb/captcha/plugins/turnstile.php @@ -31,6 +31,9 @@ class turnstile extends base /** @var string API endpoint for turnstile verification */ private const VERIFY_ENDPOINT = 'https://challenges.cloudflare.com/turnstile/v0/siteverify'; + /** @var Client */ + protected Client $client; + /** @var language */ protected language $language; @@ -141,7 +144,7 @@ class turnstile extends base ]; // Create guzzle client - $client = new Client(); + $client = $this->get_client(); // Check captcha with turnstile API try @@ -174,6 +177,21 @@ class turnstile extends base } } + /** + * Get Guzzle client + * + * @return Client + */ + protected function get_client(): Client + { + if (!isset($this->client)) + { + $this->client = new Client(); + } + + return $this->client; + } + /** * {@inheritDoc} */ diff --git a/tests/captcha/turnstile_test.php b/tests/captcha/turnstile_test.php new file mode 100644 index 0000000000..79e64e1ea0 --- /dev/null +++ b/tests/captcha/turnstile_test.php @@ -0,0 +1,254 @@ + + * @license GNU General Public License, version 2 (GPL-2.0) + * + * For full copyright and license information, please see + * the docs/CREDITS.txt file. + * + */ + +use phpbb\captcha\plugins\confirm_type; +use phpbb\captcha\plugins\turnstile; +use phpbb\config\config; +use phpbb\db\driver\driver_interface; +use phpbb\language\language; +use phpbb\log\log_interface; +use phpbb\request\request; +use phpbb\request\request_interface; +use phpbb\template\template; +use phpbb\user; +use PHPUnit\Framework\TestCase; +use GuzzleHttp\Client; +use GuzzleHttp\Psr7\Response; + +class phpbb_captcha_turnstile_test extends \phpbb_database_test_case +{ + /** @var turnstile */ + protected $turnstile; + + /** @var PHPUnit\Framework\MockObject\MockObject */ + protected $config; + + /** @var PHPUnit\Framework\MockObject\MockObject */ + protected $db; + + /** @var PHPUnit\Framework\MockObject\MockObject */ + protected $language; + + /** @var PHPUnit\Framework\MockObject\MockObject */ + protected $log; + + /** @var PHPUnit\Framework\MockObject\MockObject */ + protected $request; + + /** @var PHPUnit\Framework\MockObject\MockObject */ + protected $template; + + /** @var PHPUnit\Framework\MockObject\MockObject */ + protected $user; + + public function getDataSet() + { + return $this->createXMLDataSet(__DIR__ . '/../fixtures/empty.xml'); + } + + protected function setUp(): void + { + // Mock the dependencies + $this->config = $this->createMock(config::class); + $this->db = $this->createMock(driver_interface::class); + $this->language = $this->createMock(language::class); + $this->log = $this->createMock(log_interface::class); + $this->request = $this->createMock(request::class); + $this->template = $this->createMock(template::class); + $this->user = $this->createMock(user::class); + + $this->language->method('lang')->willReturnArgument(0); + + // Instantiate the turnstile class with the mocked dependencies + $this->turnstile = new turnstile( + $this->config, + $this->db, + $this->language, + $this->log, + $this->request, + $this->template, + $this->user + ); + } + + public function testIsAvailable(): 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->assertTrue($this->turnstile->is_available()); + } + + public function test_not_vailable(): void + { + $this->request->method('variable')->willReturnMap([ + ['confirm_id', '', false, request_interface::REQUEST, 'confirm_id'], + ['confirm_code', '', false, request_interface::REQUEST, 'confirm_code'] + ]); + + // Test when sitekey or secret is missing + $this->config->method('offsetGet')->willReturnMap([ + ['captcha_turnstile_sitekey', ''], + ['captcha_turnstile_secret', 'secret_value'], + ]); + + $this->assertFalse($this->turnstile->is_available()); + } + + public function test_get_name(): void + { + $this->assertEquals('CAPTCHA_TURNSTILE', $this->turnstile->get_name()); + } + + public function test_set_Name(): void + { + $this->turnstile->set_name('custom_service'); + $service_name_property = new \ReflectionProperty($this->turnstile, 'service_name'); + $service_name_property->setAccessible(true); + $this->assertEquals('custom_service', $service_name_property->getValue($this->turnstile)); + } + + public function test_validate_without_response(): void + { + // Test when there is no Turnstile response + $this->request->method('variable')->with('cf-turnstile-response')->willReturn(''); + + $this->assertFalse($this->turnstile->validate()); + } + + public function test_validate_with_response_success(): void + { + // Mock the request and response from the Turnstile API + $this->request->method('variable')->with('cf-turnstile-response')->willReturn('valid_response'); + $this->request->method('header')->with('CF-Connecting-IP')->willReturn('127.0.0.1'); + + // Mock the GuzzleHttp client and response + $client_mock = $this->createMock(Client::class); + $response_mock = $this->createMock(Response::class); + + $client_mock->method('request')->willReturn($response_mock); + $response_mock->method('getBody')->willReturn(json_encode(['success' => true])); + + // Mock config values for secret + $this->config->method('offsetGet')->willReturn('secret_value'); + + // Use reflection to inject the mocked client into the turnstile class + $reflection = new \ReflectionClass($this->turnstile); + $client_property = $reflection->getProperty('client'); + $client_property->setAccessible(true); + $client_property->setValue($this->turnstile, $client_mock); + + // Validate that the CAPTCHA was solved successfully + $this->assertTrue($this->turnstile->validate()); + } + + public function test_has_config(): void + { + $this->assertTrue($this->turnstile->has_config()); + } + + public function test_get_client(): void + { + $turnstile_reflection = new \ReflectionClass($this->turnstile); + $get_client_method = $turnstile_reflection->getMethod('get_client'); + $get_client_method->setAccessible(true); + $client_property = $turnstile_reflection->getProperty('client'); + $client_property->setAccessible(true); + + $this->assertFalse($client_property->isInitialized($this->turnstile)); + $client = $get_client_method->invoke($this->turnstile); + $this->assertNotNull($client); + $this->assertInstanceOf(\GuzzleHttp\Client::class, $client); + $this->assertTrue($client === $get_client_method->invoke($this->turnstile)); + } + + public function test_validate_with_response_failure(): void + { + // Mock the request and response from the Turnstile API + $this->request->method('variable')->with('cf-turnstile-response')->willReturn('valid_response'); + $this->request->method('header')->with('CF-Connecting-IP')->willReturn('127.0.0.1'); + + // Mock the GuzzleHttp client and response + $client_mock = $this->createMock(Client::class); + $response_mock = $this->createMock(Response::class); + + $client_mock->method('request')->willReturn($response_mock); + $response_mock->method('getBody')->willReturn(json_encode(['success' => false])); + + // Mock config values for secret + $this->config->method('offsetGet')->willReturn('secret_value'); + + // Use reflection to inject the mocked client into the turnstile class + $reflection = new \ReflectionClass($this->turnstile); + $client_property = $reflection->getProperty('client'); + $client_property->setAccessible(true); + $client_property->setValue($this->turnstile, $client_mock); + + // Validate that the CAPTCHA was not solved + $this->assertFalse($this->turnstile->validate()); + } + + public function test_get_template(): void + { + // Mock is_solved to return false + $is_solved_property = new \ReflectionProperty($this->turnstile, 'solved'); + $is_solved_property->setAccessible(true); + $is_solved_property->setValue($this->turnstile, false); + + // Mock the template assignments + $this->config->method('offsetGet')->willReturnMap([ + ['captcha_turnstile_sitekey', 'sitekey_value'], + ['captcha_turnstile_theme', 'light'], + ]); + + $this->request->method('variable')->willReturnMap([ + ['confirm_id', '', false, request_interface::REQUEST, 'confirm_id'], + ['confirm_code', '', false, request_interface::REQUEST, 'confirm_code'] + ]); + + $this->template->expects($this->once())->method('assign_vars')->with([ + 'S_TURNSTILE_AVAILABLE' => $this->turnstile->is_available(), + 'TURNSTILE_SITEKEY' => 'sitekey_value', + 'TURNSTILE_THEME' => 'light', + 'U_TURNSTILE_SCRIPT' => 'https://challenges.cloudflare.com/turnstile/v0/api.js', + 'CONFIRM_TYPE_REGISTRATION' => confirm_type::UNDEFINED->value, + ]); + + $this->assertEquals('captcha_turnstile.html', $this->turnstile->get_template()); + + $is_solved_property->setValue($this->turnstile, true); + $this->assertEquals('', $this->turnstile->get_template()); + } + + public function test_get_demo_template(): void + { + // Mock the config assignments + $this->config->method('offsetGet')->willReturn('light'); + + $this->template->expects($this->once())->method('assign_vars')->with([ + 'TURNSTILE_THEME' => 'light', + 'U_TURNSTILE_SCRIPT' => 'https://challenges.cloudflare.com/turnstile/v0/api.js', + ]); + + $this->assertEquals('captcha_turnstile_acp_demo.html', $this->turnstile->get_demo_template()); + } +} + From 3e938ea7653d82d87207e10e764c6e08a41bc161 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Thu, 17 Oct 2024 17:42:13 +0200 Subject: [PATCH 28/38] [ticket/17413] Add full test coverage for turnstile class PHPBB-17413 --- tests/captcha/turnstile_test.php | 176 ++++++++++++++++++++++++++++++- 1 file changed, 175 insertions(+), 1 deletion(-) diff --git a/tests/captcha/turnstile_test.php b/tests/captcha/turnstile_test.php index 79e64e1ea0..dd9b1347ed 100644 --- a/tests/captcha/turnstile_test.php +++ b/tests/captcha/turnstile_test.php @@ -15,16 +15,18 @@ use phpbb\captcha\plugins\confirm_type; use phpbb\captcha\plugins\turnstile; use phpbb\config\config; use phpbb\db\driver\driver_interface; +use phpbb\form\form_helper; use phpbb\language\language; use phpbb\log\log_interface; use phpbb\request\request; use phpbb\request\request_interface; use phpbb\template\template; use phpbb\user; -use PHPUnit\Framework\TestCase; use GuzzleHttp\Client; use GuzzleHttp\Psr7\Response; +require_once __DIR__ . '/../../phpBB/includes/functions_acp.php'; + class phpbb_captcha_turnstile_test extends \phpbb_database_test_case { /** @var turnstile */ @@ -160,6 +162,48 @@ class phpbb_captcha_turnstile_test extends \phpbb_database_test_case $this->assertTrue($this->turnstile->validate()); } + public function test_validate_with_guzzle_exception(): void + { + // Mock the request and response from the Turnstile API + $this->request->method('variable')->with('cf-turnstile-response')->willReturn('valid_response'); + $this->request->method('header')->with('CF-Connecting-IP')->willReturn('127.0.0.1'); + + // Mock the GuzzleHttp client and response + $client_mock = $this->createMock(Client::class); + + $request_mock = $this->createMock(\GuzzleHttp\Psr7\Request::class); + $exception = new \GuzzleHttp\Exception\ConnectException('Failed at connecting', $request_mock); + $client_mock->method('request')->willThrowException($exception); + + // Mock config values for secret + $this->config->method('offsetGet')->willReturn('secret_value'); + + // Use reflection to inject the mocked client into the turnstile class + $reflection = new \ReflectionClass($this->turnstile); + $client_property = $reflection->getProperty('client'); + $client_property->setAccessible(true); + $client_property->setValue($this->turnstile, $client_mock); + + // Validatation fails due to guzzle exception + $this->assertFalse($this->turnstile->validate()); + } + + public function test_validate_previous_solve(): void + { + // Use reflection to inject the mocked client into the turnstile class + $reflection = new \ReflectionClass($this->turnstile); + $confirm_id = $reflection->getProperty('confirm_id'); + $confirm_id->setValue($this->turnstile, 'confirm_id'); + $code_property = $reflection->getProperty('code'); + $code_property->setValue($this->turnstile, 'test_code'); + $confirm_code_property = $reflection->getProperty('confirm_code'); + $confirm_code_property->setValue($this->turnstile, 'test_code'); + + // Validate that the CAPTCHA was solved successfully + $this->assertTrue($this->turnstile->validate()); + $this->assertTrue($this->turnstile->is_solved()); + } + public function test_has_config(): void { $this->assertTrue($this->turnstile->has_config()); @@ -250,5 +294,135 @@ class phpbb_captcha_turnstile_test extends \phpbb_database_test_case $this->assertEquals('captcha_turnstile_acp_demo.html', $this->turnstile->get_demo_template()); } + + public function test_acp_page_display(): void + { + global $phpbb_container, $phpbb_dispatcher, $template; + + $phpbb_container = new phpbb_mock_container_builder(); + $form_helper = new form_helper($this->config, $this->request, $this->user); + $phpbb_container->set('form_helper', $form_helper); + $this->user->data['user_id'] = ANONYMOUS; + $this->user->data['user_form_salt'] = 'foobar'; + + $phpbb_dispatcher = new phpbb_mock_event_dispatcher(); + $template = $this->template; + + // Mock the template assignments + $this->config->method('offsetGet')->willReturnMap([ + ['captcha_turnstile_sitekey', 'sitekey_value'], + ['captcha_turnstile_theme', 'light'], + ]); + + $this->request->method('variable')->willReturn(''); + + $expected = [ + 1 => [ + 'TURNSTILE_THEME' => 'light', + 'U_TURNSTILE_SCRIPT' => 'https://challenges.cloudflare.com/turnstile/v0/api.js', + ], + 2 => [ + 'CAPTCHA_PREVIEW' => 'captcha_turnstile_acp_demo.html', + 'CAPTCHA_NAME' => '', + 'CAPTCHA_TURNSTILE_THEME' => 'light', + 'CAPTCHA_TURNSTILE_THEMES' => ['light', 'dark', 'auto'], + 'U_ACTION' => 'test_u_action', + ], + ]; + $matcher = $this->exactly(count($expected)); + $this->template + ->expects($matcher) + ->method('assign_vars') + ->willReturnCallback(function ($template_data) use ($matcher, $expected) { + $callNr = $matcher->getInvocationCount(); + $this->assertEquals($expected[$callNr], $template_data); + }); + + $module_mock = new ModuleMock(); + + $this->turnstile->acp_page('', $module_mock); + } + + public function test_acp_page_submit_without_form(): void + { + global $language, $phpbb_container, $phpbb_dispatcher, $template; + + $language = $this->language; + $phpbb_container = new phpbb_mock_container_builder(); + $form_helper = new form_helper($this->config, $this->request, $this->user); + $phpbb_container->set('form_helper', $form_helper); + $this->user->data['user_id'] = ANONYMOUS; + $this->user->data['user_form_salt'] = 'foobar'; + + $phpbb_dispatcher = new phpbb_mock_event_dispatcher(); + $template = $this->template; + + // Mock the template assignments + $this->config->method('offsetGet')->willReturnMap([ + ['captcha_turnstile_sitekey', 'sitekey_value'], + ['captcha_turnstile_theme', 'light'], + ]); + + $this->request->method('is_set_post')->willReturnMap([ + ['creation_time', ''], + ['submit', true] + ]); + + $this->setExpectedTriggerError(E_USER_NOTICE, 'FORM_INVALID'); + + $module_mock = new ModuleMock(); + + $this->turnstile->acp_page('', $module_mock); + } + + public function test_acp_page_submit_valid(): void + { + global $language, $phpbb_container, $phpbb_dispatcher, $template; + + $language = $this->language; + $phpbb_container = new phpbb_mock_container_builder(); + $form_helper = new form_helper($this->config, $this->request, $this->user); + $phpbb_container->set('form_helper', $form_helper); + $this->user->data['user_id'] = ANONYMOUS; + $this->user->data['user_form_salt'] = 'foobar'; + + $phpbb_dispatcher = new phpbb_mock_event_dispatcher(); + $template = $this->template; + + $form_tokens = $form_helper->get_form_tokens('acp_captcha'); + + // Mock the template assignments + $this->config->method('offsetGet')->willReturnMap([ + ['captcha_turnstile_sitekey', 'sitekey_value'], + ['captcha_turnstile_theme', 'light'], + ]); + $this->config['form_token_lifetime'] = 3600; + + $this->request->method('is_set_post')->willReturnMap([ + ['creation_time', true], + ['form_token', true], + ['submit', true] + ]); + + $this->request->method('variable')->willReturnMap([ + ['creation_time', 0, false, request_interface::REQUEST, $form_tokens['creation_time']], + ['form_token', '', false, request_interface::REQUEST, $form_tokens['form_token']], + ['captcha_turnstile_sitekey', '', false, request_interface::REQUEST, 'newsitekey'], + ['captcha_turnstile_theme', 'light', false, request_interface::REQUEST, 'auto'], + ]); + + $this->setExpectedTriggerError(E_USER_NOTICE, 'CONFIG_UPDATED'); + + $module_mock = new ModuleMock(); + sleep(1); // sleep for a second to ensure form token validation succeeds + + $this->turnstile->acp_page('', $module_mock); + } } +class ModuleMock +{ + public string $tpl_name = ''; + public string $page_title = ''; + public string $u_action = 'test_u_action'; +} From 38ce4985f47c6caf5d59a30da3b4594b2eb1846d Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Thu, 17 Oct 2024 17:43:57 +0200 Subject: [PATCH 29/38] [ticket/17414] Use language class in adm_back_link() PHPBB-17414 --- phpBB/includes/functions_acp.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/phpBB/includes/functions_acp.php b/phpBB/includes/functions_acp.php index 646d2f34d5..a3bae23ab5 100644 --- a/phpBB/includes/functions_acp.php +++ b/phpBB/includes/functions_acp.php @@ -203,8 +203,8 @@ function adm_page_footer($copyright_html = true) */ function adm_back_link($u_action) { - global $user; - return '

« ' . $user->lang['BACK_TO_PREV'] . ''; + global $language; + return '

« ' . $language->lang('BACK_TO_PREV') . ''; } /** From 1e80400d09d6042df4f7aeb8f1a9439905a6f418 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Thu, 17 Oct 2024 18:40:53 +0200 Subject: [PATCH 30/38] [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'], From dd1c6d647f326ab90871ed03ef8a35d3ea249adf Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Thu, 17 Oct 2024 19:23:04 +0200 Subject: [PATCH 31/38] [ticket/17414] Remove not needed test captchas PHPBB-17414 --- .../foo/test_captcha/captcha/test_captcha.php | 33 ------------------- .../ext/foo/test_captcha/composer.json | 23 ------------- .../ext/foo/test_captcha/config/services.yml | 5 --- 3 files changed, 61 deletions(-) delete mode 100644 tests/functional/fixtures/ext/foo/test_captcha/captcha/test_captcha.php delete mode 100644 tests/functional/fixtures/ext/foo/test_captcha/composer.json delete mode 100644 tests/functional/fixtures/ext/foo/test_captcha/config/services.yml diff --git a/tests/functional/fixtures/ext/foo/test_captcha/captcha/test_captcha.php b/tests/functional/fixtures/ext/foo/test_captcha/captcha/test_captcha.php deleted file mode 100644 index fb8c6ed090..0000000000 --- a/tests/functional/fixtures/ext/foo/test_captcha/captcha/test_captcha.php +++ /dev/null @@ -1,33 +0,0 @@ -=8.1" - }, - "extra": { - "display-name": "phpBB 4.0 Test Captcha", - "soft-require": { - "phpbb/phpbb": "4.0.*@dev" - } - } -} diff --git a/tests/functional/fixtures/ext/foo/test_captcha/config/services.yml b/tests/functional/fixtures/ext/foo/test_captcha/config/services.yml deleted file mode 100644 index 75cdf52270..0000000000 --- a/tests/functional/fixtures/ext/foo/test_captcha/config/services.yml +++ /dev/null @@ -1,5 +0,0 @@ -services: - foo_test_captcha.captcha.test_captcha: - class: foo\test_captcha\captcha\test_captcha - tags: - - { name: captcha.plugins } From 687448be49a6d3a7d0ede2aeb42ab2d4b639999e Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Thu, 17 Oct 2024 19:57:31 +0200 Subject: [PATCH 32/38] [ticket/17414] Clean up small code issues noticed by sniffer PHPBB-17414 --- phpBB/phpbb/captcha/plugins/base.php | 11 +++++++++++ phpBB/phpbb/captcha/plugins/legacy_wrapper.php | 2 +- phpBB/phpbb/captcha/plugins/turnstile.php | 2 +- 3 files changed, 13 insertions(+), 2 deletions(-) diff --git a/phpBB/phpbb/captcha/plugins/base.php b/phpBB/phpbb/captcha/plugins/base.php index 43d24f7c8d..1b02d63166 100644 --- a/phpBB/phpbb/captcha/plugins/base.php +++ b/phpBB/phpbb/captcha/plugins/base.php @@ -1,4 +1,15 @@ + * @license GNU General Public License, version 2 (GPL-2.0) + * + * For full copyright and license information, please see + * the docs/CREDITS.txt file. + * + */ namespace phpbb\captcha\plugins; diff --git a/phpBB/phpbb/captcha/plugins/legacy_wrapper.php b/phpBB/phpbb/captcha/plugins/legacy_wrapper.php index d728a05aba..abb26b34ba 100644 --- a/phpBB/phpbb/captcha/plugins/legacy_wrapper.php +++ b/phpBB/phpbb/captcha/plugins/legacy_wrapper.php @@ -65,7 +65,7 @@ class legacy_wrapper implements plugin_interface return $this->legacy_captcha->has_config(); } - return false; + return ''; } /** diff --git a/phpBB/phpbb/captcha/plugins/turnstile.php b/phpBB/phpbb/captcha/plugins/turnstile.php index bf9ada4831..edd6832159 100644 --- a/phpBB/phpbb/captcha/plugins/turnstile.php +++ b/phpBB/phpbb/captcha/plugins/turnstile.php @@ -272,7 +272,7 @@ class turnstile extends base { foreach ($captcha_vars as $captcha_var => $template_var) { - $var = $this->request->is_set($captcha_var) ? $this->request->variable($captcha_var, '') : $this->config->offsetGet($captcha_var);; + $var = $this->request->is_set($captcha_var) ? $this->request->variable($captcha_var, '') : $this->config->offsetGet($captcha_var); $this->template->assign_var($template_var, $var); } From 28f597e1826bec448a93b59a53ca3e219e176a4a Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Sat, 19 Oct 2024 15:48:46 +0200 Subject: [PATCH 33/38] [ticket/17413] Use lang function in turnstile captcha PHPBB-17413 --- phpBB/styles/prosilver/template/captcha_turnstile.html | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/phpBB/styles/prosilver/template/captcha_turnstile.html b/phpBB/styles/prosilver/template/captcha_turnstile.html index aed3f9299c..1cf4f4c792 100644 --- a/phpBB/styles/prosilver/template/captcha_turnstile.html +++ b/phpBB/styles/prosilver/template/captcha_turnstile.html @@ -1,8 +1,7 @@ {% if CONFIRM_TYPE_REGISTRATION %}
- -

{L_CONFIRMATION}

+

{{ lang('CONFIRMATION') }}

{% endif %} {% if S_TURNSTILE_AVAILABLE %} From fa66bc5150cdaa29d6556d7d81a78e7d8ce90a3a Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Sat, 19 Oct 2024 16:55:53 +0200 Subject: [PATCH 34/38] [ticket/17414] Fix small issues with compatibility to legacy captchas PHPBB-17414 --- phpBB/phpbb/captcha/plugins/legacy_wrapper.php | 2 +- phpBB/phpbb/captcha/plugins/qa.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/phpBB/phpbb/captcha/plugins/legacy_wrapper.php b/phpBB/phpbb/captcha/plugins/legacy_wrapper.php index abb26b34ba..8f8642b2b2 100644 --- a/phpBB/phpbb/captcha/plugins/legacy_wrapper.php +++ b/phpBB/phpbb/captcha/plugins/legacy_wrapper.php @@ -86,7 +86,7 @@ class legacy_wrapper implements plugin_interface { if (method_exists($this->legacy_captcha, 'init')) { - $this->legacy_captcha->init($type); + $this->legacy_captcha->init($type->value); } } diff --git a/phpBB/phpbb/captcha/plugins/qa.php b/phpBB/phpbb/captcha/plugins/qa.php index 3e48df00b2..6e09aaaf13 100644 --- a/phpBB/phpbb/captcha/plugins/qa.php +++ b/phpBB/phpbb/captcha/plugins/qa.php @@ -40,7 +40,7 @@ class qa protected $service_name; /** @var int Question ID */ - protected $question = -1; + private $question = -1; /** * Constructor From 08ab64a3b20e6f9cd211d03ce9d87f4d3b2eda5d Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Sun, 20 Oct 2024 09:04:36 +0200 Subject: [PATCH 35/38] [ticket/17414] Improve docblocks in legacy wrapper PHPBB-17414 --- phpBB/phpbb/captcha/plugins/legacy_wrapper.php | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/phpBB/phpbb/captcha/plugins/legacy_wrapper.php b/phpBB/phpbb/captcha/plugins/legacy_wrapper.php index 8f8642b2b2..cc0609f4b0 100644 --- a/phpBB/phpbb/captcha/plugins/legacy_wrapper.php +++ b/phpBB/phpbb/captcha/plugins/legacy_wrapper.php @@ -15,8 +15,10 @@ namespace phpbb\captcha\plugins; class legacy_wrapper implements plugin_interface { + /** @var object Legacy CAPTCHA instance, should implement functionality as required in phpBB 3.3 */ private $legacy_captcha; + /** @var string Last error */ private string $last_error; /** @@ -206,6 +208,9 @@ class legacy_wrapper implements plugin_interface } } + /** + * {@inheritDoc} + */ public function acp_page($id, $module): void { if (method_exists($this->legacy_captcha, 'acp_page')) From fd994f4742f095020657cc9bba15640fc7bde63f Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Sun, 20 Oct 2024 09:43:45 +0200 Subject: [PATCH 36/38] [ticket/17414] Add unit test for legacy wrapper PHPBB-17414 --- .../phpbb/captcha/plugins/legacy_wrapper.php | 2 +- tests/captcha/legacy_wrapper_test.php | 261 ++++++++++++++++++ 2 files changed, 262 insertions(+), 1 deletion(-) create mode 100644 tests/captcha/legacy_wrapper_test.php diff --git a/phpBB/phpbb/captcha/plugins/legacy_wrapper.php b/phpBB/phpbb/captcha/plugins/legacy_wrapper.php index cc0609f4b0..600683194d 100644 --- a/phpBB/phpbb/captcha/plugins/legacy_wrapper.php +++ b/phpBB/phpbb/captcha/plugins/legacy_wrapper.php @@ -64,7 +64,7 @@ class legacy_wrapper implements plugin_interface { if (method_exists($this->legacy_captcha, 'get_name')) { - return $this->legacy_captcha->has_config(); + return $this->legacy_captcha->get_name(); } return ''; diff --git a/tests/captcha/legacy_wrapper_test.php b/tests/captcha/legacy_wrapper_test.php new file mode 100644 index 0000000000..873012bd6a --- /dev/null +++ b/tests/captcha/legacy_wrapper_test.php @@ -0,0 +1,261 @@ + + * @license GNU General Public License, version 2 (GPL-2.0) + * + * For full copyright and license information, please see + * the docs/CREDITS.txt file. + * + */ + +use phpbb\captcha\plugins\confirm_type; +use phpbb\captcha\plugins\legacy_wrapper; + +class phpbb_captcha_legacy_wrapper_test extends phpbb_test_case +{ + private $legacy_captcha; + private $legacy_wrapper; + + public function setUp(): void + { + $this->legacy_captcha = $this->createMock(stdClass::class); + $this->legacy_wrapper = new legacy_wrapper($this->legacy_captcha); + } + + public function test_is_available_with_method_exists(): void + { + // Simulate is_available method exists in the legacy captcha + $this->legacy_captcha = $this->getMockBuilder(stdClass::class) + ->addMethods(['is_available']) + ->getMock(); + $this->legacy_captcha->method('is_available')->willReturn(true); + $this->legacy_wrapper = new legacy_wrapper($this->legacy_captcha); + + $this->assertTrue($this->legacy_wrapper->is_available()); + } + + public function test_is_available_without_method_exists(): void + { + // Simulate is_available method does not exist in the legacy captcha + $this->assertFalse($this->legacy_wrapper->is_available()); + } + + public function test_has_config_with_method_exists(): void + { + // Simulate has_config method exists in the legacy captcha + $this->legacy_captcha = $this->getMockBuilder(stdClass::class) + ->addMethods(['has_config']) + ->getMock(); + $this->legacy_wrapper = new legacy_wrapper($this->legacy_captcha); + $this->legacy_captcha->method('has_config')->willReturn(true); + + $this->assertTrue($this->legacy_wrapper->has_config()); + } + + public function test_has_config_without_method_exists(): void + { + // Simulate has_config method does not exist in the legacy captcha + $this->assertFalse($this->legacy_wrapper->has_config()); + } + + public function test_get_name_with_method_exists(): void + { + // Simulate get_name method exists in the legacy captcha + $this->legacy_captcha = $this->getMockBuilder(stdClass::class) + ->addMethods(['get_name']) + ->getMock(); + $this->legacy_wrapper = new legacy_wrapper($this->legacy_captcha); + $this->legacy_captcha->method('get_name')->willReturn('LegacyCaptchaName'); + + $this->assertSame('LegacyCaptchaName', $this->legacy_wrapper->get_name()); + } + + public function test_get_name_without_method_exists(): void + { + // Simulate get_name method does not exist in the legacy captcha + $this->assertSame('', $this->legacy_wrapper->get_name()); + } + + public function test_set_name_with_method_exists(): void + { + // Simulate set_name method exists in the legacy captcha + $this->legacy_captcha = $this->getMockBuilder(stdClass::class) + ->addMethods(['set_name']) + ->getMock(); + $this->legacy_wrapper = new legacy_wrapper($this->legacy_captcha); + $this->legacy_captcha->expects($this->once())->method('set_name')->with('NewName'); + + $this->legacy_wrapper->set_name('NewName'); + } + + public function test_init_with_method_exists(): void + { + // Simulate init method exists in the legacy captcha + $this->legacy_captcha = $this->getMockBuilder(stdClass::class) + ->addMethods(['init']) + ->getMock(); + $this->legacy_wrapper = new legacy_wrapper($this->legacy_captcha); + $this->legacy_captcha->expects($this->once())->method('init')->with(confirm_type::REGISTRATION->value); + + $this->legacy_wrapper->init(confirm_type::REGISTRATION); + } + + public function test_get_hidden_fields_with_method_exists(): void + { + // Simulate get_hidden_fields method exists in the legacy captcha + $this->legacy_captcha = $this->getMockBuilder(stdClass::class) + ->addMethods(['get_hidden_fields']) + ->getMock(); + $this->legacy_wrapper = new legacy_wrapper($this->legacy_captcha); + $this->legacy_captcha->method('get_hidden_fields')->willReturn(['field1' => 'value1']); + + $this->assertSame(['field1' => 'value1'], $this->legacy_wrapper->get_hidden_fields()); + } + + public function test_get_hidden_fields_without_method_exists(): void + { + // Simulate get_hidden_fields method does not exist in the legacy captcha + $this->assertSame([], $this->legacy_wrapper->get_hidden_fields()); + } + + public function test_validate_with_error(): void + { + // Simulate validate method returns an error + $this->legacy_captcha = $this->getMockBuilder(stdClass::class) + ->addMethods(['validate']) + ->getMock(); + $this->legacy_wrapper = new legacy_wrapper($this->legacy_captcha); + $this->legacy_captcha->method('validate')->willReturn('Captcha Error'); + + $this->assertFalse($this->legacy_wrapper->validate()); + $this->assertSame('Captcha Error', $this->legacy_wrapper->get_error()); + } + + public function test_validate_without_method_exists(): void + { + $this->assertFalse($this->legacy_wrapper->validate()); + } + + public function test_validate_without_error(): void + { + // Simulate validate method does not return an error + $this->legacy_captcha = $this->getMockBuilder(stdClass::class) + ->addMethods(['validate']) + ->getMock(); + $this->legacy_wrapper = new legacy_wrapper($this->legacy_captcha); + $this->legacy_captcha->method('validate')->willReturn(null); + + $this->assertTrue($this->legacy_wrapper->validate()); + } + + public function test_is_solved_with_method_exists(): void + { + // Simulate is_solved method exists in the legacy captcha + $this->legacy_captcha = $this->getMockBuilder(stdClass::class) + ->addMethods(['is_solved']) + ->getMock(); + $this->legacy_wrapper = new legacy_wrapper($this->legacy_captcha); + $this->legacy_captcha->method('is_solved')->willReturn(true); + + $this->assertTrue($this->legacy_wrapper->is_solved()); + } + + public function test_is_solved_without_method_exists(): void + { + // Simulate is_solved method does not exist in the legacy captcha + $this->assertFalse($this->legacy_wrapper->is_solved()); + } + + public function test_reset_with_method_exists(): void + { + // Simulate reset method exists in the legacy captcha + $this->legacy_captcha = $this->getMockBuilder(stdClass::class) + ->addMethods(['reset']) + ->getMock(); + $this->legacy_wrapper = new legacy_wrapper($this->legacy_captcha); + $this->legacy_captcha->expects($this->once())->method('reset'); + + $this->legacy_wrapper->reset(); + } + + public function test_get_attempt_count_with_method_exists(): void + { + // Simulate get_attempt_count method exists in the legacy captcha + $this->legacy_captcha = $this->getMockBuilder(stdClass::class) + ->addMethods(['get_attempt_count']) + ->getMock(); + $this->legacy_wrapper = new legacy_wrapper($this->legacy_captcha); + $this->legacy_captcha->method('get_attempt_count')->willReturn(5); + + $this->assertSame(5, $this->legacy_wrapper->get_attempt_count()); + } + + public function test_get_attempt_count_without_method_exists(): void + { + // Simulate get_attempt_count method does not exist in the legacy captcha + $this->assertSame(PHP_INT_MAX, $this->legacy_wrapper->get_attempt_count()); + } + + public function test_get_template_with_method_exists(): void + { + // Simulate get_template method exists in the legacy captcha + $this->legacy_captcha = $this->getMockBuilder(stdClass::class) + ->addMethods(['get_template']) + ->getMock(); + $this->legacy_wrapper = new legacy_wrapper($this->legacy_captcha); + $this->legacy_captcha->method('get_template')->willReturn('template_content'); + + $this->assertSame('template_content', $this->legacy_wrapper->get_template()); + } + + public function test_get_template_without_method_exists(): void + { + // Simulate get_template method does not exist in the legacy captcha + $this->assertSame('', $this->legacy_wrapper->get_template()); + } + + public function test_get_demo_template_with_method_exists(): void + { + // Simulate get_demo_template method exists in the legacy captcha + $this->legacy_captcha = $this->getMockBuilder(stdClass::class) + ->addMethods(['get_demo_template']) + ->getMock(); + $this->legacy_wrapper = new legacy_wrapper($this->legacy_captcha); + $this->legacy_captcha->method('get_demo_template')->willReturn('demo_template_content'); + + $this->assertSame('demo_template_content', $this->legacy_wrapper->get_demo_template()); + } + + public function test_get_demo_template_without_method_exists(): void + { + // Simulate get_demo_template method does not exist in the legacy captcha + $this->assertSame('', $this->legacy_wrapper->get_demo_template()); + } + + public function test_garbage_collect_with_method_exists(): void + { + // Simulate garbage_collect method exists in the legacy captcha + $this->legacy_captcha = $this->getMockBuilder(stdClass::class) + ->addMethods(['garbage_collect']) + ->getMock(); + $this->legacy_wrapper = new legacy_wrapper($this->legacy_captcha); + $this->legacy_captcha->expects($this->once())->method('garbage_collect')->with(confirm_type::REGISTRATION->value); + + $this->legacy_wrapper->garbage_collect(confirm_type::REGISTRATION); + } + + public function test_acp_page_with_method_exists(): void + { + // Simulate acp_page method exists in the legacy captcha + $this->legacy_captcha = $this->getMockBuilder(stdClass::class) + ->addMethods(['acp_page']) + ->getMock(); + $this->legacy_wrapper = new legacy_wrapper($this->legacy_captcha); + $this->legacy_captcha->expects($this->once())->method('acp_page')->with(1, 'module'); + + $this->legacy_wrapper->acp_page(1, 'module'); + } +} From 3b27b65c760c4a16ea542c943a9211693ac1b222 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Sun, 20 Oct 2024 10:55:15 +0200 Subject: [PATCH 37/38] [ticket/17414] Change incomplete CAPTCHA to extend new base class PHPBB-17414 --- .../default/container/services_captcha.yml | 4 + phpBB/phpbb/captcha/plugins/base.php | 7 ++ phpBB/phpbb/captcha/plugins/incomplete.php | 94 +++++++++---------- .../phpbb/captcha/plugins/legacy_wrapper.php | 2 +- .../captcha/plugins/plugin_interface.php | 2 +- phpBB/phpbb/captcha/plugins/turnstile.php | 2 +- tests/captcha/incomplete_test.php | 41 +++++--- 7 files changed, 86 insertions(+), 66 deletions(-) diff --git a/phpBB/config/default/container/services_captcha.yml b/phpBB/config/default/container/services_captcha.yml index d4f1ad863a..ea41fefe80 100644 --- a/phpBB/config/default/container/services_captcha.yml +++ b/phpBB/config/default/container/services_captcha.yml @@ -19,7 +19,11 @@ services: shared: false arguments: - '@config' + - '@dbal.conn' + - '@language' + - '@request' - '@template' + - '@user' - '%core.root_path%' - '%core.php_ext%' calls: diff --git a/phpBB/phpbb/captcha/plugins/base.php b/phpBB/phpbb/captcha/plugins/base.php index 1b02d63166..a30e679731 100644 --- a/phpBB/phpbb/captcha/plugins/base.php +++ b/phpBB/phpbb/captcha/plugins/base.php @@ -246,4 +246,11 @@ abstract class base implements plugin_interface } $this->db->sql_freeresult($result); } + + /** + * {@inheritDoc} + */ + public function acp_page(mixed $id, mixed $module): void + { + } } diff --git a/phpBB/phpbb/captcha/plugins/incomplete.php b/phpBB/phpbb/captcha/plugins/incomplete.php index ec3376c0a5..07998a7125 100644 --- a/phpBB/phpbb/captcha/plugins/incomplete.php +++ b/phpBB/phpbb/captcha/plugins/incomplete.php @@ -14,25 +14,34 @@ namespace phpbb\captcha\plugins; use phpbb\config\config; -use phpbb\exception\runtime_exception; +use phpbb\db\driver\driver_interface; +use phpbb\language\language; +use phpbb\request\request_interface; use phpbb\template\template; +use phpbb\user; -class incomplete extends captcha_abstract +class incomplete extends base { /** * Constructor for incomplete captcha * * @param config $config + * @param driver_interface $db + * @param language $language + * @param request_interface $request * @param template $template + * @param user $user * @param string $phpbb_root_path * @param string $phpEx */ - public function __construct(protected config $config, protected template $template, - protected string $phpbb_root_path, protected string $phpEx) - {} + public function __construct(config $config, driver_interface $db, language $language, request_interface $request, + protected template $template, user $user, protected string $phpbb_root_path, protected string $phpEx) + { + parent::__construct($config, $db, $language, $request, $user); + } /** - * @return bool True if captcha is available, false if not + * {@inheritDoc} */ public function is_available(): bool { @@ -40,70 +49,45 @@ class incomplete extends captcha_abstract } /** - * Dummy implementation, not supported by this captcha - * - * @throws runtime_exception - * @return void + * {@inheritDoc} */ - public function get_generator_class(): void + public function has_config(): bool { - throw new runtime_exception('NO_GENERATOR_CLASS'); + return false; } /** - * Get CAPTCHA name language variable - * - * @return string Language variable + * {@inheritDoc} */ - public static function get_name(): string + public function get_name(): string { return 'CAPTCHA_INCOMPLETE'; } /** - * Init CAPTCHA - * - * @param int $type CAPTCHA type - * @return void + * {@inheritDoc} */ - public function init($type) + public function set_name(string $name): void { } /** - * Execute demo - * - * @return void + * {@inheritDoc} */ - public function execute_demo() + public function init(confirm_type $type): void { } /** - * Execute CAPTCHA - * - * @return void + * {@inheritDoc} */ - public function execute() - { - } - - /** - * Get template data for demo - * - * @param int|string $id ACP module ID - * - * @return string Demo template file name - */ - public function get_demo_template($id): string + public function get_demo_template(): string { return ''; } /** - * Get template data for CAPTCHA - * - * @return string CAPTCHA template file name + * {@inheritDoc} */ public function get_template(): string { @@ -118,9 +102,7 @@ class incomplete extends captcha_abstract } /** - * Validate CAPTCHA - * - * @return false Incomplete CAPTCHA will never validate + * {@inheritDoc} */ public function validate(): bool { @@ -128,12 +110,26 @@ class incomplete extends captcha_abstract } /** - * Check whether CAPTCHA is solved - * - * @return false Incomplete CAPTCHA will never be solved + * {@inheritDoc} + */ + public function get_error(): string + { + return ''; + } + + /** + * {@inheritDoc} */ public function is_solved(): bool { return false; } + + /** + * {@inheritDoc} + */ + public function get_attempt_count(): int + { + return 0; + } } diff --git a/phpBB/phpbb/captcha/plugins/legacy_wrapper.php b/phpBB/phpbb/captcha/plugins/legacy_wrapper.php index 600683194d..aaabddb48b 100644 --- a/phpBB/phpbb/captcha/plugins/legacy_wrapper.php +++ b/phpBB/phpbb/captcha/plugins/legacy_wrapper.php @@ -211,7 +211,7 @@ class legacy_wrapper implements plugin_interface /** * {@inheritDoc} */ - public function acp_page($id, $module): void + public function acp_page(mixed $id, mixed $module): void { if (method_exists($this->legacy_captcha, 'acp_page')) { diff --git a/phpBB/phpbb/captcha/plugins/plugin_interface.php b/phpBB/phpbb/captcha/plugins/plugin_interface.php index f9dd9c6402..41483e45a5 100644 --- a/phpBB/phpbb/captcha/plugins/plugin_interface.php +++ b/phpBB/phpbb/captcha/plugins/plugin_interface.php @@ -122,5 +122,5 @@ interface plugin_interface * @param mixed $module ACP module name * @return void */ - public function acp_page($id, $module): void; + public function acp_page(mixed $id, mixed $module): void; } diff --git a/phpBB/phpbb/captcha/plugins/turnstile.php b/phpBB/phpbb/captcha/plugins/turnstile.php index edd6832159..afd7a363b2 100644 --- a/phpBB/phpbb/captcha/plugins/turnstile.php +++ b/phpBB/phpbb/captcha/plugins/turnstile.php @@ -229,7 +229,7 @@ class turnstile extends base /** * {@inheritDoc} */ - public function acp_page($id, $module): void + public function acp_page(mixed $id, mixed $module): void { $captcha_vars = [ 'captcha_turnstile_sitekey' => 'CAPTCHA_TURNSTILE_SITEKEY', diff --git a/tests/captcha/incomplete_test.php b/tests/captcha/incomplete_test.php index c1da2b6c48..4c5e7c6b27 100644 --- a/tests/captcha/incomplete_test.php +++ b/tests/captcha/incomplete_test.php @@ -11,11 +11,15 @@ * */ +use phpbb\captcha\plugins\confirm_type; use phpbb\captcha\plugins\incomplete; use phpbb\config\config; +use phpbb\language\language; +use phpbb\request\request; use phpbb\template\template; +use phpbb\user; -class phpbb_captcha_incomplete_test extends phpbb_test_case +class phpbb_captcha_incomplete_test extends phpbb_database_test_case { protected config $config; @@ -32,21 +36,34 @@ class phpbb_captcha_incomplete_test extends phpbb_test_case $this->assigned_vars = array_merge($this->assigned_vars, $vars); } + public function getDataSet() + { + return $this->createXMLDataSet(__DIR__ . '/../fixtures/empty.xml'); + } + protected function setUp(): void { global $phpbb_root_path, $phpEx; $this->config = new config([]); $this->template = $this->getMockBuilder('\phpbb\template\twig\twig') - ->setMethods(['assign_vars']) + ->onlyMethods(['assign_vars']) ->disableOriginalConstructor() ->getMock(); $this->template->method('assign_vars') ->willReturnCallback([$this, 'assign_vars']); + $db = $this->new_dbal(); + $language = $this->createMock(language::class); + $request = $this->createMock(request::class); + $user = $this->createMock(user::class); $this->incomplete_captcha = new incomplete( $this->config, + $db, + $language, + $request, $this->template, + $user, $phpbb_root_path, $phpEx ); @@ -57,29 +74,25 @@ class phpbb_captcha_incomplete_test extends phpbb_test_case $this->assertTrue($this->incomplete_captcha->is_available()); $this->assertFalse($this->incomplete_captcha->is_solved()); $this->assertFalse($this->incomplete_captcha->validate()); - $this->assertSame('CAPTCHA_INCOMPLETE', incomplete::get_name()); - $this->incomplete_captcha->init(0); - $this->incomplete_captcha->execute(); - $this->incomplete_captcha->execute_demo(); + $this->assertFalse($this->incomplete_captcha->has_config()); + $this->incomplete_captcha->set_name('foo'); + $this->assertSame('CAPTCHA_INCOMPLETE', $this->incomplete_captcha->get_name()); + $this->incomplete_captcha->init(confirm_type::UNDEFINED); $this->assertEmpty($this->assigned_vars); $this->assertEmpty($this->incomplete_captcha->get_demo_template(0)); - } - - public function test_get_generator_class(): void - { - $this->expectException(\phpbb\exception\runtime_exception::class); - $this->incomplete_captcha->get_generator_class(); + $this->assertEmpty($this->incomplete_captcha->get_error()); + $this->assertSame(0, $this->incomplete_captcha->get_attempt_count()); } public function test_get_tempate(): void { - $this->incomplete_captcha->init(CONFIRM_REG); + $this->incomplete_captcha->init(confirm_type::REGISTRATION); $this->assertSame('captcha_incomplete.html', $this->incomplete_captcha->get_template()); $this->assertEquals('CONFIRM_INCOMPLETE', $this->assigned_vars['CONFIRM_LANG']); $this->assigned_vars = []; - $this->incomplete_captcha->init(CONFIRM_POST); + $this->incomplete_captcha->init(confirm_type::POST); $this->assertSame('captcha_incomplete.html', $this->incomplete_captcha->get_template()); $this->assertEquals('CONFIRM_INCOMPLETE', $this->assigned_vars['CONFIRM_LANG']); } From dd49bb4a0a6835f207f91875024185782304d057 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Tue, 22 Oct 2024 19:00:17 +0200 Subject: [PATCH 38/38] [ticket/17414] Remove idempotency and use user ip for check PHPBB-17414 --- phpBB/phpbb/captcha/plugins/turnstile.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/phpBB/phpbb/captcha/plugins/turnstile.php b/phpBB/phpbb/captcha/plugins/turnstile.php index afd7a363b2..9a235edc3b 100644 --- a/phpBB/phpbb/captcha/plugins/turnstile.php +++ b/phpBB/phpbb/captcha/plugins/turnstile.php @@ -139,8 +139,7 @@ class turnstile extends base $form_data = [ 'secret' => $this->config['captcha_turnstile_secret'], 'response' => $turnstile_response, - 'remoteip' => $this->request->header('CF-Connecting-IP'), - //'idempotency_key' => $this->confirm_id, // check if we need this + 'remoteip' => $this->user->ip, ]; // Create guzzle client