[ticket/17413] Clean up turnstile code and add attempt counting

PHPBB-17413

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 : '',
+		];
 	}
 
 	/**

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(),
+			'S_TURNSTILE_AVAILABLE'		=> $this->is_available(),
-			'TURNSTILE_SITEKEY'		=> $this->config->offsetGet('captcha_turnstile_sitekey'),
+			'TURNSTILE_SITEKEY'			=> $this->config->offsetGet('captcha_turnstile_sitekey'),
-			'TURNSTILE_THEME'		=> $this->config->offsetGet('captcha_turnstile_theme'),
+			'TURNSTILE_THEME'			=> $this->config->offsetGet('captcha_turnstile_theme'),
-			'U_TURNSTILE_SCRIPT'	=> self::SCRIPT_URL,
+			'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}
 	 */

phpBB/styles/prosilver/template/captcha_turnstile.html

@@ -1,12 +1,24 @@
+{% if CONFIRM_TYPE_REGISTRATION %}
+<div class="panel captcha-panel">
+	<div class="inner">
+
+		<h3 class="captcha-title">{L_CONFIRMATION}</h3>
+		<fieldset class="fields2">
+{% endif %}
 {% if S_TURNSTILE_AVAILABLE %}
-	<noscript>
+			<noscript>
-		<div>{{ lang('CAPTCHA_TURNSTILE_NOSCRIPT') }}</div>
+				<div>{{ lang('CAPTCHA_TURNSTILE_NOSCRIPT') }}</div>
-	</noscript>
+			</noscript>
 
-	<script src="{{ U_TURNSTILE_SCRIPT }}" async defer></script>
+			<script src="{{ U_TURNSTILE_SCRIPT }}" async defer></script>
 
-	{# The cf-turnstile class is used in JavaScript #}
+			{# The cf-turnstile class is used in JavaScript #}
-	<div class="cf-turnstile" data-language="{{ lang('TURNSTILE_LANG') }}" data-sitekey="{{ TURNSTILE_SITEKEY }}"{% if TURNSTILE_THEME %} data-theme="{{ TURNSTILE_THEME }}"{% endif %}></div>
+			<div class="cf-turnstile" data-language="{{ lang('TURNSTILE_LANG') }}" data-sitekey="{{ TURNSTILE_SITEKEY }}"{% if TURNSTILE_THEME %} data-theme="{{ TURNSTILE_THEME }}"{% endif %}></div>
 {% else %}
 	{{ lang('CAPTCHA_TURNSTILE_NOT_AVAILABLE') }}
 {% endif %}
+{% if CONFIRM_TYPE_REGISTRATION %}
+		</fieldset>
+	</div>
+</div>
+{% endif %}