ENH Use symfony/validation logic

silverstripe · Sep 24, 2024 · 34db9ce · 34db9ce
1 parent 7793664
commit 34db9ce
Show file tree

Hide file tree

Showing 25 changed files with 443 additions and 284 deletions.
diff --git a/_config/backtrace.yml b/_config/backtrace.yml
@@ -28,7 +28,8 @@ SilverStripe\Dev\Backtrace:
     - ['SilverStripe\Security\PasswordEncryptor_Blowfish', 'encryptA']
     - ['SilverStripe\Security\PasswordEncryptor_Blowfish', 'encryptX']
     - ['SilverStripe\Security\PasswordEncryptor_Blowfish', 'encryptY']
-    - ['SilverStripe\Security\PasswordValidator', 'validate']
+    - ['SilverStripe\Security\Validation\RulesPasswordValidator', 'validate']
+    - ['SilverStripe\Security\Validation\EntropyPasswordValidator', 'validate']
     - ['SilverStripe\Security\RememberLoginHash', 'setToken']
     - ['SilverStripe\Security\Security', 'encrypt_password']
     - ['*', 'checkPassword']

diff --git a/_config/passwords.yml b/_config/passwords.yml
@@ -2,12 +2,5 @@
 Name: corepasswords
 ---
 SilverStripe\Core\Injector\Injector:
-  SilverStripe\Security\PasswordValidator:
-    properties:
-      MinLength: 8
-      HistoricCount: 6
-
-# In the case someone uses `new PasswordValidator` instead of Injector, provide some safe defaults through config.
-SilverStripe\Security\PasswordValidator:
-  min_length: 8
-  historic_count: 6
+  SilverStripe\Security\Validation\PasswordValidator:
+    class: 'SilverStripe\Security\Validation\EntropyPasswordValidator'
diff --git a/composer.json b/composer.json
@@ -48,7 +48,7 @@
         "symfony/mailer": "^7.0",
         "symfony/mime": "^7.0",
         "symfony/translation": "^7.0",
-        "symfony/validator": "^7.0",
+        "symfony/validator": "^7.1",
         "symfony/yaml": "^7.0",
         "ext-ctype": "*",
         "ext-dom": "*",

diff --git a/src/Control/Email/Email.php b/src/Control/Email/Email.php
@@ -4,14 +4,13 @@
 
 use Exception;
 use RuntimeException;
-use Egulias\EmailValidator\EmailValidator;
-use Egulias\EmailValidator\Validation\RFCValidation;
 use SilverStripe\Control\Director;
 use SilverStripe\Core\Config\Configurable;
 use SilverStripe\Core\Environment;
 use SilverStripe\Core\Extensible;
 use SilverStripe\Core\Injector\Injectable;
 use SilverStripe\Core\Injector\Injector;
+use SilverStripe\Core\Validation\ConstraintValidator;
 use SilverStripe\ORM\FieldType\DBField;
 use SilverStripe\View\ArrayData;
 use SilverStripe\View\Requirements;
@@ -22,6 +21,8 @@
 use Symfony\Component\Mime\Address;
 use Symfony\Component\Mime\Email as SymfonyEmail;
 use Symfony\Component\Mime\Part\AbstractPart;
+use Symfony\Component\Validator\Constraints\Email as EmailConstraint;
+use Symfony\Component\Validator\Constraints\NotBlank;
 
 class Email extends SymfonyEmail
 {
@@ -63,16 +64,17 @@ class Email extends SymfonyEmail
     private bool $dataHasBeenSet = false;
 
     /**
-     * Checks for RFC822-valid email format.
-     *
-     * @copyright Cal Henderson <[email protected]>
-     *    This code is licensed under a Creative Commons Attribution-ShareAlike 2.5 License
-     *    http://creativecommons.org/licenses/by-sa/2.5/
+     * Checks for RFC valid email format.
      */
     public static function is_valid_address(string $address): bool
     {
-        $validator = new EmailValidator();
-        return $validator->isValid($address, new RFCValidation());
+        return ConstraintValidator::validate(
+            $address,
+            [
+                new EmailConstraint(mode: EmailConstraint::VALIDATION_MODE_STRICT),
+                new NotBlank()
+            ]
+        )->isValid();
     }
 
     public static function getSendAllEmailsTo(): array
@@ -117,7 +119,7 @@ private static function convertConfigToAddreses(array|string $config): array
         $addresses = [];
         if (is_array($config)) {
             foreach ($config as $key => $val) {
-                if (filter_var($key, FILTER_VALIDATE_EMAIL)) {
+                if (static::is_valid_address($key)) {
                     $addresses[] = new Address($key, $val);
                 } else {
                     $addresses[] = new Address($val);

diff --git a/src/Control/HTTPRequest.php b/src/Control/HTTPRequest.php
@@ -7,6 +7,8 @@
 use InvalidArgumentException;
 use SilverStripe\Core\ClassInfo;
 use SilverStripe\ORM\ArrayLib;
+use Symfony\Component\Validator\Constraints\Ip;
+use Symfony\Component\Validator\Constraints\IpValidator;
 
 /**
  * Represents a HTTP-request, including a URL that is tokenised for parsing, and a request method
@@ -810,7 +812,8 @@ public function getIP()
      */
     public function setIP($ip)
     {
-        if (!filter_var($ip, FILTER_VALIDATE_IP)) {
+        // We can't use ConstraintValidator here because it relies on injector and the kernel may not have booted yet.
+        if (!IpValidator::checkIp($ip, Ip::ALL)) {
             throw new InvalidArgumentException("Invalid ip $ip");
         }
         $this->ip = $ip;

diff --git a/src/Control/Middleware/TrustedProxyMiddleware.php b/src/Control/Middleware/TrustedProxyMiddleware.php
@@ -3,7 +3,10 @@
 namespace SilverStripe\Control\Middleware;
 
 use SilverStripe\Control\HTTPRequest;
+use SilverStripe\Core\Validation\ConstraintValidator;
 use Symfony\Component\HttpFoundation\IpUtils;
+use Symfony\Component\Validator\Constraints\Ip;
+use Symfony\Component\Validator\Constraints\NotBlank;
 
 /**
  * This middleware will rewrite headers that provide IP and host details from an upstream proxy.
@@ -220,14 +223,14 @@ protected function getIPFromHeaderValue($headerValue)
 
         // Prioritise filters
         $filters = [
-            FILTER_FLAG_NO_PRIV_RANGE | FILTER_FLAG_NO_RES_RANGE,
-            FILTER_FLAG_NO_PRIV_RANGE,
-            null
+            Ip::ALL_ONLY_PUBLIC,
+            Ip::ALL_NO_PRIVATE,
+            Ip::ALL
         ];
         foreach ($filters as $filter) {
             // Find best IP
             foreach ($ips as $ip) {
-                if (filter_var($ip, FILTER_VALIDATE_IP, $filter ?? 0)) {
+                if (ConstraintValidator::validate($ip, [new Ip(version: $filter), new NotBlank()])->isValid()) {
                     return $ip;
                 }
             }

diff --git a/src/Core/Convert.php b/src/Core/Convert.php
@@ -2,9 +2,12 @@
 
 namespace SilverStripe\Core;
 
+use SilverStripe\Core\Validation\ConstraintValidator;
 use SimpleXMLElement;
 use SilverStripe\ORM\DB;
 use SilverStripe\View\Parsers\URLSegmentFilter;
+use Symfony\Component\Validator\Constraints\NotBlank;
+use Symfony\Component\Validator\Constraints\Url;
 
 /**
  * Library of conversion functions, implemented as static methods.
@@ -226,16 +229,14 @@ public static function xml2raw($val)
 
     /**
      * Create a link if the string is a valid URL
-     *
-     * @param string $string The string to linkify
-     * @return string A link to the URL if string is a URL
      */
-    public static function linkIfMatch($string)
-    {
-        if (preg_match('/^[a-z+]+\:\/\/[a-zA-Z0-9$-_.+?&=!*\'()%]+$/', $string ?? '')) {
+    public static function linkIfMatch(
+        string $string,
+        array $protocols = ['file', 'ftp', 'http', 'https', 'imap', 'nntp']
+    ): string {
+        if (ConstraintValidator::validate($string, [new Url(protocols: $protocols), new NotBlank()])->isValid()) {
             return "<a style=\"white-space: nowrap\" href=\"$string\">$string</a>";
         }
-
         return $string;
     }
 

diff --git a/src/Forms/EmailField.php b/src/Forms/EmailField.php
@@ -2,52 +2,40 @@
 
 namespace SilverStripe\Forms;
 
+use SilverStripe\Core\Validation\ConstraintValidator;
+use Symfony\Component\Validator\Constraints\Email as EmailConstraint;
+
 /**
- * Text input field with validation for correct email format according to RFC 2822.
+ * Text input field with validation for correct email format according to the relevant RFC.
  */
 class EmailField extends TextField
 {
-
     protected $inputType = 'email';
-    /**
-     * {@inheritdoc}
-     */
+
     public function Type()
     {
         return 'email text';
     }
 
     /**
-     * Validates for RFC 2822 compliant email addresses.
-     *
-     * @see http://www.regular-expressions.info/email.html
-     * @see http://www.ietf.org/rfc/rfc2822.txt
+     * Validates for RFC compliant email addresses.
      *
      * @param Validator $validator
-     *
-     * @return string
      */
     public function validate($validator)
     {
-        $result = true;
         $this->value = trim($this->value ?? '');
 
-        $pattern = '^[a-z0-9!#$%&\'*+/=?^_`{|}~-]+(?:\\.[a-z0-9!#$%&\'*+/=?^_`{|}~-]+)*@(?:[a-z0-9](?:[a-z0-9-]*[a-z0-9])?\\.)+[a-z0-9](?:[a-z0-9-]*[a-z0-9])?$';
-
-        // Escape delimiter characters.
-        $safePattern = str_replace('/', '\\/', $pattern ?? '');
-
-        if ($this->value && !preg_match('/' . $safePattern . '/i', $this->value ?? '')) {
-            $validator->validationError(
-                $this->name,
-                _t('SilverStripe\\Forms\\EmailField.VALIDATION', 'Please enter an email address'),
-                'validation'
-            );
-
-            $result = false;
-        }
+        $message = _t('SilverStripe\\Forms\\EmailField.VALIDATION', 'Please enter an email address');
+        $result = ConstraintValidator::validate(
+            $this->value,
+            new EmailConstraint(message: $message, mode: EmailConstraint::VALIDATION_MODE_STRICT),
+            $this->getName()
+        );
+        $validator->getResult()->combineAnd($result);
+        $isValid = $result->isValid();
 
-        return $this->extendValidationResult($result, $validator);
+        return $this->extendValidationResult($isValid, $validator);
     }
 
     public function getSchemaValidation()

diff --git a/src/Forms/UrlField.php b/src/Forms/UrlField.php
@@ -7,26 +7,89 @@
 
 /**
  * Text input field with validation for a url
- * Url must include a scheme, either http:// or https://
+ * Url must include a protocol (aka scheme) such as https:// or http://
  */
 class UrlField extends TextField
 {
+    /**
+     * The default set of protocols allowed for valid URLs
+     */
+    private static array $default_protocols = ['https', 'http'];
+
+    /**
+     * The default value for whether a relative protocol (// on its own) is allowed
+     */
+    private static bool $default_allow_relative_protocol = false;
+
+    private array $protocols = [];
+
+    private ?bool $allowRelativeProtocol = null;
+
     public function Type()
     {
         return 'text url';
     }
 
     public function validate($validator)
     {
-        $result = true;
-        if ($this->value && !ConstraintValidator::validate($this->value, new Url())->isValid()) {
-            $validator->validationError(
-                $this->name,
-                _t(__CLASS__ . '.INVALID', 'Please enter a valid URL'),
-                'validation'
-            );
-            $result = false;
+        $allowedProtocols = $this->getAllowedProtocols();
+        $message = _t(
+            __CLASS__ . '.INVALID_WITH_PROTOCOL',
+            'Please enter a valid URL including a protocol, e.g {protocol}://example.com',
+            ['protocol' => $allowedProtocols[0]]
+        );
+        $result = ConstraintValidator::validate(
+            $this->value,
+            new Url(
+                message: $message,
+                protocols: $allowedProtocols,
+                relativeProtocol: $this->getAllowRelativeProtocol()
+            ),
+            $this->getName()
+        );
+        $validator->getResult()->combineAnd($result);
+        $isValid = $result->isValid();
+        return $this->extendValidationResult($isValid, $validator);
+    }
+
+    /**
+     * Set which protocols valid URLs are allowed to have
+     */
+    public function setAllowedProtocols(array $protocols): static
+    {
+        // Ensure the array isn't associative so we can use 0 index in validate().
+        $this->protocols = array_keys($protocols);
+        return $this;
+    }
+
+    /**
+     * Get which protocols valid URLs are allowed to have
+     */
+    public function getAllowedProtocols(): array
+    {
+        if (empty($this->protocols)) {
+            return static::config()->get('default_protocols');
+        }
+        return $this->protocols;
+    }
+
+    /**
+     * Set whether a relative protocol (// on its own) is allowed
+     */
+    public function setAllowRelativeProtocol(?bool $allow): static
+    {
+        $this->allowRelativeProtocol = $allow;
+        return $this;
+    }
+
+    /**
+     * Get whether a relative protocol (// on its own) is allowed
+     */
+    public function getAllowRelativeProtocol(): bool
+    {
+        if ($this->allowRelativeProtocol === null) {
+            return static::config()->get('default_allow_relative_protocol');
         }
-        return $this->extendValidationResult($result, $validator);
+        return $this->allowRelativeProtocol;
     }
 }
diff --git a/src/Security/Member.php b/src/Security/Member.php
@@ -44,6 +44,8 @@
 use SilverStripe\Forms\SearchableDropdownField;
 use SilverStripe\Forms\SearchableMultiDropdownField;
 use SilverStripe\ORM\FieldType\DBForeignKey;
+use SilverStripe\Security\Validation\PasswordValidator;
+use SilverStripe\Security\Validation\RulesPasswordValidator;
 
 /**
  * The member class which represents the users of the system
@@ -378,10 +380,8 @@ public function isLockedOut()
 
     /**
      * Set a {@link PasswordValidator} object to use to validate member's passwords.
-     *
-     * @param PasswordValidator $validator
      */
-    public static function set_password_validator(PasswordValidator $validator = null)
+    public static function set_password_validator(?PasswordValidator $validator = null)
     {
         // Override existing config
         Config::modify()->remove(Injector::class, PasswordValidator::class);
@@ -394,10 +394,8 @@ public static function set_password_validator(PasswordValidator $validator = nul
 
     /**
      * Returns the default {@link PasswordValidator}
-     *
-     * @return PasswordValidator|null
      */
-    public static function password_validator()
+    public static function password_validator(): ?PasswordValidator
     {
         if (Injector::inst()->has(PasswordValidator::class)) {
             return Injector::inst()->get(PasswordValidator::class);
@@ -1763,11 +1761,16 @@ public function generateRandomPassword(int $length = 0): string
     {
         $password = '';
         $validator = Member::password_validator();
-        if ($length && $validator && $length < $validator->getMinLength()) {
-            throw new InvalidArgumentException('length argument is less than password validator minLength');
+        if ($validator instanceof RulesPasswordValidator) {
+            $validatorMinLength = $validator->getMinLength();
+            if ($length && $length < $validatorMinLength) {
+                throw new InvalidArgumentException('length argument is less than password validator minLength');
+            }
+        } else {
+            // Make sure the password is long enough to beat even very strict entropy tests
+            $validatorMinLength = 128;
         }
-        $validatorMinLength = $validator ? $validator->getMinLength() : 0;
-        $len = $length ?: max($validatorMinLength, 20);
+        $len = max($length, $validatorMinLength, 20);
         // The default PasswordValidator checks the password includes the following four character sets
         $charsets = [
             'abcdefghijklmnopqrstuvwyxz',