Improve sanitization routine for Settings objects

Ensure type safety for values stored in Settings.
chesio · Mar 4, 2024 · 0c7b11d · 0c7b11d
1 parent 4a702d2
commit 0c7b11d
Show file tree

Hide file tree

Showing 6 changed files with 23 additions and 34 deletions.
diff --git a/classes/BlueChip/Security/Core/Settings.php b/classes/BlueChip/Security/Core/Settings.php
@@ -233,13 +233,16 @@ public function sanitize(array $settings, array $defaults = []): array
         // Loop over default values instead of provided $settings - this way only known keys are preserved.
         foreach ($values as $key => $default_value) {
             if (isset($settings[$key])) {
-                // New value is provided, sanitize it either...
-                $values[$key] = isset(static::SANITIZERS[$key])
-                    // ...using provided callback...
-                    ? \call_user_func(static::SANITIZERS[$key], $settings[$key], $default_value)
-                    // ...or by type.
-                    : self::sanitizeByType($settings[$key], $default_value)
-                ;
+                // Sanitize the value by type first (= ensure the value has expected type).
+                $value = self::sanitizeByType($settings[$key], $default_value);
+
+                // If custom sanitizer for this setting key is provided...
+                if (isset(static::SANITIZERS[$key])) {
+                    // ...execute it on type-safe value.
+                    $value = \call_user_func(static::SANITIZERS[$key], $value, $default_value);
+                }
+
+                $values[$key] = $value;
             }
         }
 
@@ -249,8 +252,13 @@ public function sanitize(array $settings, array $defaults = []): array
 
     /**
      * Sanitize the $value according to type of $default value.
+     *
+     * @param mixed $value
+     * @param mixed[]|bool|float|int|string $default
+     *
+     * @return mixed[]|bool|float|int|string
      */
-    protected static function sanitizeByType(mixed $value, mixed $default): mixed
+    protected static function sanitizeByType(mixed $value, array|bool|float|int|string $default): array|bool|float|int|string
     {
         if (\is_bool($default)) {
             return (bool) $value;

diff --git a/classes/BlueChip/Security/Modules/BadRequestsBanner/Settings.php b/classes/BlueChip/Security/Modules/BadRequestsBanner/Settings.php
@@ -51,13 +51,6 @@ class Settings extends CoreSettings
         self::BAN_DURATION => 60,
     ];
 
-    /**
-     * @var array<string,callable> Custom sanitizers.
-     */
-    protected const SANITIZERS = [
-        self::BAD_REQUEST_PATTERNS => [self::class, 'parseList'],
-    ];
-
 
     /**
      * @return BanRule[] List of active ban rules.

diff --git a/classes/BlueChip/Security/Modules/ExternalBlocklist/Settings.php b/classes/BlueChip/Security/Modules/ExternalBlocklist/Settings.php
@@ -26,15 +26,8 @@ class Settings extends CoreSettings
 
     /**
      * Sanitize lock scope values. Allow only expected values.
-     *
-     * @internal Form data are submitted as string, so always accept string type for $value.
-     *
-     * @param int|string $value
-     * @param int $default
-     *
-     * @return int
      */
-    public static function sanitizeAccessScope(int|string $value, int $default): int
+    public static function sanitizeAccessScope(int $value, int $default): int
     {
         return Scope::tryFrom((int) $value) ? ((int) $value) : $default;
     }

diff --git a/classes/BlueChip/Security/Modules/Login/Settings.php b/classes/BlueChip/Security/Modules/Login/Settings.php
@@ -71,13 +71,13 @@ class Settings extends CoreSettings
     /**
      * Sanitize "username blacklist" setting. Must be list of valid usernames.
      *
-     * @param string|string[] $value
+     * @param string[] $value
      *
      * @return string[]
      */
-    public static function sanitizeUsernameBlacklist(array|string $value): array
+    public static function sanitizeUsernameBlacklist(array $value): array
     {
-        return \array_filter(self::parseList($value), '\validate_username');
+        return \array_filter($value, '\validate_username');
     }
 
 

diff --git a/classes/BlueChip/Security/Modules/Notifications/Settings.php b/classes/BlueChip/Security/Modules/Notifications/Settings.php
@@ -82,12 +82,12 @@ class Settings extends CoreSettings
     /**
      * Sanitize "notification recipients" setting. Must be list of emails.
      *
-     * @param string|string[] $value
+     * @param string[] $value
      *
      * @return string[]
      */
-    public static function sanitizeNotificationRecipient(array|string $value): array
+    public static function sanitizeNotificationRecipient(array $value): array
     {
-        return \array_filter(self::parseList($value), '\is_email');
+        return \array_filter($value, '\is_email');
     }
 }
diff --git a/classes/BlueChip/Security/Setup/Settings.php b/classes/BlueChip/Security/Setup/Settings.php
@@ -40,11 +40,6 @@ class Settings extends CoreSettings
 
     /**
      * Sanitize connection type. Allow only expected values.
-     *
-     * @param string $value
-     * @param string $default
-     *
-     * @return string
      */
     public static function sanitizeConnectionType(string $value, string $default): string
     {