Validation report fixes

DOCUMENTATION.md

@@ -122,7 +122,7 @@ $accepted = $consent_manager->register(string $id, array $definition);
 - Registration IDs and script IDs may only use letters, numbers, `.`, `_`, `:`, and `-`, and must start with a letter or number.
 - Supported categories are `necessary`, `analytics`, `marketing`, and `media`.
 - Each `scripts` definition must use **one** of these execution sources: `src`, `asset`, or `inline`.
-- `src` accepts `http`, `https`, or relative URLs. URLs such as `//example.com/...` are not allowed.
+- `src` accepts HTTPS or relative URLs. URLs such as `//example.com/...` are not allowed.
 - `asset` must be a local phpBB asset path such as `@vendor_example/js/file.js`.
 - Unsafe HTML event-handler attributes such as `onclick` are ignored.
 
@@ -172,7 +172,7 @@ Each entry inside `scripts` supports the following options.
 |----------------------|------------------------------|-------------------------------------------------------------------------------------------------------------------------|---------------------------------------------------|
 | `id`                 | Optional                     | Unique ID for this script. If omitted, Consent Manager creates one when needed.                                         | `'id' => 'vendor.example.analytics.loader'`       |
 | `category`           | Optional                     | Lets this script use a different category from the main registration. Most extensions should keep the same category.    | `'category' => 'marketing'`                       |
-| `src`                | Needed for remote files      | URL of an external script, or a relative URL. Do not combine with `asset` or `inline`.                                  | `'src' => 'https://cdn.example.com/analytics.js'` |
+| `src`                | Needed for remote files      | HTTPS URL of an external script, or a path relative to the phpBB root. Do not combine with `asset` or `inline`.         | `'src' => 'https://cdn.example.com/analytics.js'` |
 | `asset`              | Needed for extension files   | Local phpBB asset path. Best for JavaScript files that ship with your extension. Do not combine with `src` or `inline`. | `'asset' => '@vendor_example/js/analytics.js'`    |
 | `inline`             | Needed for inline JavaScript | JavaScript code that Consent Manager should inject after consent. Do not combine with `src` or `asset`.                 | `'inline' => 'window.tracker.start();'`           |
 | `async`              | Optional                     | Sets `<script async>`. Defaults to `true` for `src` and `asset`, and `false` for `inline`.                              | `'async' => true`                                 |

controller/acp_controller.php

@@ -94,17 +94,23 @@ public function handle()
 		{
 			$this->validate_form_key('phpbb_consentmanager_acp');
 
-			$errors = [];
-			$saved = $this->acp_manager->save_settings([
+			$submitted_settings = [
 				'analytics_enabled' => $this->request->variable('consentmanager_analytics_enabled', 0),
 				'marketing_enabled' => $this->request->variable('consentmanager_marketing_enabled', 0),
 				'media_enabled' => $this->request->variable('consentmanager_media_enabled', 0),
-				'integrations' => trim($this->request->raw_variable('consentmanager_integrations', '')),
+				'integrations' => $this->request->raw_variable('consentmanager_integrations', ''),
+			];
+			$errors = [];
+			$saved = $this->acp_manager->save_settings([
+				'analytics_enabled' => $submitted_settings['analytics_enabled'],
+				'marketing_enabled' => $submitted_settings['marketing_enabled'],
+				'media_enabled' => $submitted_settings['media_enabled'],
+				'integrations' => trim($submitted_settings['integrations']),
 			], $errors);
 
 			if (!$saved)
 			{
-				$this->assign_template_vars($errors);
+				$this->assign_template_vars($errors, $submitted_settings);
 				return;
 			}
 
@@ -347,10 +353,21 @@ protected function get_current_form_token_fields()
 		];
 	}
 
-	protected function assign_template_vars(array $errors = [])
+	protected function assign_template_vars(array $errors = [], array $submitted_settings = null)
 	{
+		$template_data = $this->acp_manager->get_settings_template_data();
+		if ($submitted_settings !== null)
+		{
+			$template_data = array_merge($template_data, [
+				'S_CONSENTMANAGER_ANALYTICS' => !empty($submitted_settings['analytics_enabled']),
+				'S_CONSENTMANAGER_MARKETING' => !empty($submitted_settings['marketing_enabled']),
+				'S_CONSENTMANAGER_MEDIA' => !empty($submitted_settings['media_enabled']),
+				'CONSENTMANAGER_INTEGRATIONS' => (string) $submitted_settings['integrations'],
+			]);
+		}
+
 		$this->template->assign_vars(array_merge(
-			$this->acp_manager->get_settings_template_data(),
+			$template_data,
 			[
 				'S_ERROR'	=> !empty($errors),
 				'ERROR_MSG'	=> implode('<br>', $errors),

controller/log_controller.php

@@ -64,10 +64,11 @@ public function log(Request $request)
 			], $this->get_error_status_code($submission['error']));
 		}
 
-		$this->log_manager->log_consent($submission['categories'], $submission['version']);
+		$logged = $this->log_manager->log_consent($submission['categories'], $submission['version']);
 
 		return new JsonResponse([
 			'success' => true,
+			'logged' => $logged,
 			'categories' => $submission['categories'],
 			'version' => $submission['version'],
 		]);

language/en/acp_consentmanager.php

@@ -33,7 +33,7 @@
 	'ACP_CONSENTMANAGER_INTEGRATIONS_REQUIRED'		=> 'Required properties',
 	'ACP_CONSENTMANAGER_INTEGRATIONS_REQUIRED_ID'	=> 'may only use letters, numbers, dots, underscores, colons, and hyphens.',
 	'ACP_CONSENTMANAGER_INTEGRATIONS_REQUIRED_CAT'	=> 'must be one of these values:',
-	'ACP_CONSENTMANAGER_INTEGRATIONS_REQUIRED_SRC'	=> 'must be a valid HTTPS or relative script URL.',
+	'ACP_CONSENTMANAGER_INTEGRATIONS_REQUIRED_SRC'	=> 'must be an absolute HTTPS URL.',
 	'ACP_CONSENTMANAGER_INTEGRATIONS_OPTIONAL'		=> 'Optional properties',
 	'ACP_CONSENTMANAGER_INTEGRATIONS_EXAMPLE_LABEL'	=> 'Example Analytics',
 	'ACP_CONSENTMANAGER_INTEGRATIONS_EXAMPLE_DESC'	=> 'Loads a simple analytics library after consent.',
@@ -45,7 +45,7 @@
 	'ACP_CONSENTMANAGER_FORCE_REPROMPT'				=> 'Force re-prompt',
 	'ACP_CONSENTMANAGER_REPROMPT_SUCCESS'			=> 'Consent version increased. Visitors will be asked to review their settings again.',
 	'ACP_CONSENTMANAGER_INVALID_INTEGRATIONS'		=> 'The integrations field must contain a valid JSON array.',
-	'ACP_CONSENTMANAGER_INVALID_INTEGRATION_ENTRY'	=> 'Integration entry %1$s is invalid. Each entry must include a safe id, supported category, and valid script source URL.',
+	'ACP_CONSENTMANAGER_INVALID_INTEGRATION_ENTRY'	=> 'Integration entry %1$s is invalid. Each entry must include a safe id, supported category, and absolute HTTPS script source URL.',
 	'ACP_CONSENTMANAGER_INVALID_JSON'				=> 'Invalid JSON',
 	'ACP_CONSENTMANAGER_BANNER_EXPLAIN'				=> 'Customise or translate the default text shown in the consent banner and privacy settings dialog for each installed language. BBCode and URLs are supported.',
 	'ACP_CONSENTMANAGER_BANNER_TITLE'				=> 'Banner title',

migrations/m5_hmac_secret.php

@@ -25,7 +25,7 @@ public static function depends_on()
 	public function update_data()
 	{
 		return [
-			['config.add', ['consentmanager_hmac_secret', (string) $this->config['rand_seed']]],
+			['config.add', ['consentmanager_hmac_secret', bin2hex(random_bytes(32))]],
 		];
 	}
 

service/consent_manager.php

@@ -492,7 +492,7 @@ public function normalize_integrations($input, array &$errors = [])
 			$category = isset($item['category']) ? trim((string) $item['category']) : '';
 			$src = isset($item['src']) ? trim((string) $item['src']) : '';
 
-			if (!$this->is_valid_identifier($id) || !$this->is_supported_category($category) || !$this->is_valid_script_source($src))
+			if (!$this->is_valid_identifier($id) || !$this->is_supported_category($category) || !$this->is_valid_absolute_https_script_source($src))
 			{
 				$errors[] = $this->language->lang('ACP_CONSENTMANAGER_INVALID_INTEGRATION_ENTRY', $index + 1);
 				continue;
@@ -1026,6 +1026,28 @@ protected function is_valid_script_source($src)
 		return !isset($parts['scheme']) || strtolower($parts['scheme']) === 'https';
 	}
 
+	/**
+	 * Determine whether an ACP-managed source is an absolute HTTPS URL.
+	 *
+	 * @param string $src Script source URL
+	 *
+	 * @return bool
+	 */
+	protected function is_valid_absolute_https_script_source($src)
+	{
+		if (!$this->is_valid_script_source($src))
+		{
+			return false;
+		}
+
+		$parts = parse_url($src);
+
+		return $parts !== false
+			&& isset($parts['scheme'], $parts['host'])
+			&& strtolower($parts['scheme']) === 'https'
+			&& $parts['host'] !== '';
+	}
+
 	/**
 	 * Resolve a local asset path to a frontend URL.
 	 *

service/log_manager.php

@@ -22,6 +22,9 @@ class log_manager
 	/** Maximum accepted decisions per anonymized subject during the rate-limit window. */
 	public const RATE_LIMIT_MAX = 20;
 
+	/** Maximum accepted guest decisions per IP address during the rate-limit window. */
+	public const IP_RATE_LIMIT_MAX = 200;
+
 	/** Rate-limit window in seconds. */
 	public const RATE_LIMIT_WINDOW = 3600;
 
@@ -68,7 +71,7 @@ public function log_consent(array $categories, $version)
 		$accepted_categories = json_encode(array_values($categories));
 		$now = time();
 
-		if ($this->should_suppress_submission($throttle_id, (int) $version, $accepted_categories, $now))
+		if ($this->should_suppress_submission($anonymized_id, $throttle_id, (int) $version, $accepted_categories, $now))
 		{
 			return false;
 		}
@@ -90,18 +93,19 @@ public function log_consent(array $categories, $version)
 	/**
 	 * Suppress rapid duplicates and excessive submissions from one subject.
 	 *
+	 * @param string $anonymized_id Anonymized user or guest-session identifier
 	 * @param string $throttle_id Anonymized user or guest-IP throttle identifier
 	 * @param int    $version Consent version
 	 * @param string $accepted_categories JSON-encoded normalized categories
 	 * @param int    $now Current Unix timestamp
 	 *
 	 * @return bool
 	 */
-	protected function should_suppress_submission($throttle_id, $version, $accepted_categories, $now)
+	protected function should_suppress_submission($anonymized_id, $throttle_id, $version, $accepted_categories, $now)
 	{
 		$sql = 'SELECT consent_version, accepted_categories, consent_time
 			FROM ' . $this->consent_logs_table . "
-			WHERE throttle_id = '" . $this->db->sql_escape($throttle_id) . "'
+			WHERE anonymized_id = '" . $this->db->sql_escape($anonymized_id) . "'
 				AND consent_time >= " . ((int) $now - self::RATE_LIMIT_WINDOW) . '
 			ORDER BY consent_log_id DESC';
 		$result = $this->db->sql_query_limit($sql, self::RATE_LIMIT_MAX);
@@ -126,7 +130,30 @@ protected function should_suppress_submission($throttle_id, $version, $accepted_
 			return true;
 		}
 
-		return $count >= self::RATE_LIMIT_MAX;
+		if ($count >= self::RATE_LIMIT_MAX)
+		{
+			return true;
+		}
+
+		if ($throttle_id === $anonymized_id)
+		{
+			return false;
+		}
+
+		$sql = 'SELECT consent_log_id
+			FROM ' . $this->consent_logs_table . "
+			WHERE throttle_id = '" . $this->db->sql_escape($throttle_id) . "'
+				AND consent_time >= " . ((int) $now - self::RATE_LIMIT_WINDOW);
+		$result = $this->db->sql_query_limit($sql, self::IP_RATE_LIMIT_MAX);
+		$count = 0;
+
+		while ($this->db->sql_fetchrow($result))
+		{
+			$count++;
+		}
+		$this->db->sql_freeresult($result);
+
+		return $count >= self::IP_RATE_LIMIT_MAX;
 	}
 
 	/**

service/translation_manager.php

@@ -188,6 +188,9 @@ public function save_translations(array $submitted_translations, array $allowed_
 		$allowed_languages = array_fill_keys($installed_languages, true);
 		$allowed_keys = array_intersect($allowed_keys, array_keys(self::BANNER_FIELDS));
 		$allowed_keys = array_fill_keys($allowed_keys, true);
+		$delete_keys_by_language = [];
+		$replacement_rows = [];
+		$updated_at = time();
 
 		foreach ($submitted_translations as $lang_iso => $translations)
 		{
@@ -202,11 +205,11 @@ public function save_translations(array $submitted_translations, array $allowed_
 				{
 					continue;
 				}
+				$delete_keys_by_language[$lang_iso][$translation_key] = true;
 
 				$translation_text = trim((string) $translation_text);
 				if ($translation_text === '' || $translation_text === $this->get_language_default($lang_iso, self::BANNER_FIELDS[$translation_key]['fallback']))
 				{
-					$this->delete_translation($translation_key, $lang_iso);
 					continue;
 				}
 
@@ -227,7 +230,16 @@ public function save_translations(array $submitted_translations, array $allowed_
 					continue;
 				}
 
-				$this->upsert_translation($translation_key, $lang_iso, $translation_text, $parsed_text, $uid, $bitfield, $options);
+				$replacement_rows[] = [
+					'translation_key' => $translation_key,
+					'lang_iso' => $lang_iso,
+					'translation_text' => utf8_encode_ucr($translation_text),
+					'translation_text_parsed' => $parsed_text,
+					'translation_uid' => $uid,
+					'translation_bitfield' => $bitfield,
+					'translation_options' => (int) $options,
+					'updated_at' => $updated_at,
+				];
 			}
 		}
 
@@ -236,6 +248,26 @@ public function save_translations(array $submitted_translations, array $allowed_
 			return false;
 		}
 
+		if (!empty($delete_keys_by_language))
+		{
+			$delete_conditions = [];
+			foreach ($delete_keys_by_language as $lang_iso => $translation_keys)
+			{
+				$delete_conditions[] = "(lang_iso = '" . $this->db->sql_escape($lang_iso) . "'
+					AND " . $this->db->sql_in_set('translation_key', array_keys($translation_keys)) . ')';
+			}
+
+			$this->db->sql_transaction('begin');
+			$this->db->sql_query('DELETE FROM ' . $this->translations_table . '
+				WHERE ' . implode(' OR ', $delete_conditions));
+
+			if (!empty($replacement_rows))
+			{
+				$this->db->sql_multi_insert($this->translations_table, $replacement_rows);
+			}
+			$this->db->sql_transaction('commit');
+		}
+
 		$this->translations = null;
 		$this->consent_cache->invalidate_translations();
 
@@ -314,62 +346,6 @@ protected function get_custom_translation_row($translation_key, $lang_iso)
 		return $translations[$translation_key][$lang_iso] ?? null;
 	}
 
-	/**
-	 * Insert or update a custom translation.
-	 *
-	 * @return void
-	 */
-	protected function upsert_translation($translation_key, $lang_iso, $translation_text, $parsed_text, $uid, $bitfield, $options)
-	{
-		$sql_ary = [
-			'translation_key' => $translation_key,
-			'lang_iso' => $lang_iso,
-			'translation_text' => utf8_encode_ucr($translation_text),
-			'translation_text_parsed' => $parsed_text,
-			'translation_uid' => $uid,
-			'translation_bitfield' => $bitfield,
-			'translation_options' => (int) $options,
-			'updated_at' => time(),
-		];
-
-		$sql = 'SELECT translation_id
-			FROM ' . $this->translations_table . "
-			WHERE translation_key = '" . $this->db->sql_escape($translation_key) . "'
-				AND lang_iso = '" . $this->db->sql_escape($lang_iso) . "'";
-		$result = $this->db->sql_query($sql);
-		$translation_id = (int) $this->db->sql_fetchfield('translation_id');
-		$this->db->sql_freeresult($result);
-
-		if ($translation_id)
-		{
-			$sql = 'UPDATE ' . $this->translations_table . '
-				SET ' . $this->db->sql_build_array('UPDATE', $sql_ary) . '
-				WHERE translation_id = ' . $translation_id;
-		}
-		else
-		{
-			$sql = 'INSERT INTO ' . $this->translations_table . ' ' . $this->db->sql_build_array('INSERT', $sql_ary);
-		}
-
-		$this->db->sql_query($sql);
-	}
-
-	/**
-	 * Delete a custom translation.
-	 *
-	 * @param string $translation_key Translation key
-	 * @param string $lang_iso Language ISO
-	 *
-	 * @return void
-	 */
-	protected function delete_translation($translation_key, $lang_iso)
-	{
-		$sql = 'DELETE FROM ' . $this->translations_table . "
-			WHERE translation_key = '" . $this->db->sql_escape($translation_key) . "'
-				AND lang_iso = '" . $this->db->sql_escape($lang_iso) . "'";
-		$this->db->sql_query($sql);
-	}
-
 	/**
 	 * Return an extension language-file default for a specific language.
 	 *

tests/controller/acp_controller_test.php

@@ -145,7 +145,11 @@ public function test_handle_submit_validation_errors_reassigns_form_data()
 			return $vars['S_ERROR']
 				&& $vars['ERROR_MSG'] === 'Invalid integrations'
 				&& $vars['U_ACTION'] === self::ACP_URL
-				&& isset($vars['CONSENTMANAGER_VERSION']);
+				&& isset($vars['CONSENTMANAGER_VERSION'])
+				&& $vars['S_CONSENTMANAGER_ANALYTICS'] === true
+				&& $vars['S_CONSENTMANAGER_MARKETING'] === false
+				&& $vars['S_CONSENTMANAGER_MEDIA'] === false
+				&& $vars['CONSENTMANAGER_INTEGRATIONS'] === "  invalid json  \n";
 		})];
 
 		$this->template->expects(self::once())