From a8420a5b1f0e7c7db35a3b466efb7771ef3623e2 Mon Sep 17 00:00:00 2001 From: Jim Graham Date: Thu, 26 Sep 2024 13:19:05 -0400 Subject: [PATCH] Fix User Date of Birth Validation and Processing (#16620) ### What does it do? Replace usage of `strtotime` with `DateTimeImmutable::createFromFormat` for better format compatibility. ### Why is it needed? Certain date formats, such as m-d-Y, are incompatible with `strtotime` and will cause the value of input fields where it's currently used (such as in a User's Profile) to fail. ### How to test Set the system setting `manager_date_format` to `m-d-Y` (or any other format where the month and day would be programmatically ambiguous), clear caches, and verify that a User's DOB can be added/updated as expected. Note that there are two distinct places to check this: under Manage/Users/[User] and via the logged in User's Profile page (at `/manager/?a=security/profile`) ### Related issue(s)/PR(s) n/a --- .../Processors/Security/Profile/Update.php | 20 ++- .../Processors/Security/User/Validation.php | 139 ++++++++++-------- 2 files changed, 88 insertions(+), 71 deletions(-) diff --git a/core/src/Revolution/Processors/Security/Profile/Update.php b/core/src/Revolution/Processors/Security/Profile/Update.php index cd7337fd57e..be44e4fd40d 100644 --- a/core/src/Revolution/Processors/Security/Profile/Update.php +++ b/core/src/Revolution/Processors/Security/Profile/Update.php @@ -1,4 +1,5 @@ profile === null) { return $this->modx->lexicon('user_profile_err_not_found'); } - return true; } @@ -96,12 +95,19 @@ public function prepare() /* format and set data */ $dob = $this->getProperty('dob'); if (!empty($dob)) { - $properties['dob'] = strtotime($dob); + $date = \DateTimeImmutable::createFromFormat($this->modx->getOption('manager_date_format', null, 'Y-m-d', true), $dob); + if ($date === false) { + $this->addFieldError('dob', $this->modx->lexicon('user_err_not_specified_dob')); + } else { + $properties['dob'] = $date->getTimestamp(); + } } + $this->profile->fromArray($properties); } - public function validate() { + public function validate() + { if ($this->getProperty('newpassword') !== 'false') { $oldPassword = $this->getProperty('password_old'); $newPassword = $this->getProperty('password_new'); @@ -113,10 +119,10 @@ public function validate() { } if (empty($newPassword) || strlen($newPassword) < $this->modx->getOption('password_min_length', null, 8)) { $this->addFieldError('password_new', $this->modx->lexicon('user_err_password_too_short')); - } else if (!preg_match('/^[^\'\x3c\x3e\(\);\x22\x7b\x7d\x2f\x5c]+$/', $newPassword)) { + } elseif (!preg_match('/^[^\'\x3c\x3e\(\);\x22\x7b\x7d\x2f\x5c]+$/', $newPassword)) { $this->addFieldError('password_new', $this->modx->lexicon('user_err_password_invalid')); } - if (empty($confirmPassword) || strcmp($newPassword,$confirmPassword) != 0) { + if (empty($confirmPassword) || strcmp($newPassword, $confirmPassword) != 0) { $this->addFieldError('password_confirm', $this->modx->lexicon('user_err_password_no_match')); } } diff --git a/core/src/Revolution/Processors/Security/User/Validation.php b/core/src/Revolution/Processors/Security/User/Validation.php index bb789ef763d..e1d27fb6bc4 100644 --- a/core/src/Revolution/Processors/Security/User/Validation.php +++ b/core/src/Revolution/Processors/Security/User/Validation.php @@ -1,4 +1,5 @@ processor =& $processor; $this->modx =& $processor->modx; $this->user =& $user; $this->profile =& $profile; } - public function validate() { + public function validate() + { $this->checkUsername(); $this->checkPassword(); $this->checkEmail(); @@ -50,57 +53,60 @@ public function validate() { return !$this->processor->hasErrors(); } - public function checkUsername() { + public function checkUsername() + { $username = $this->processor->getProperty('username'); if (empty($username)) { - $this->processor->addFieldError('username',$this->modx->lexicon('user_err_not_specified_username')); + $this->processor->addFieldError('username', $this->modx->lexicon('user_err_not_specified_username')); } elseif (!preg_match('/^[^\'\\x3c\\x3e\\(\\);\\x22]+$/', $username)) { - $this->processor->addFieldError('username',$this->modx->lexicon('user_err_username_invalid')); - } else if (!empty($username)) { + $this->processor->addFieldError('username', $this->modx->lexicon('user_err_username_invalid')); + } elseif (!empty($username)) { if ($this->alreadyExists($username)) { - $this->processor->addFieldError('username',$this->modx->lexicon('user_err_already_exists')); + $this->processor->addFieldError('username', $this->modx->lexicon('user_err_already_exists')); } - $this->user->set('username',$username); + $this->user->set('username', $username); } } - public function alreadyExists($name) { - return $this->modx->getCount(modUser::class, - [ + public function alreadyExists($name) + { + return $this->modx->getCount( + modUser::class, + [ 'username' => $name, 'id:!=' => $this->user->get('id'), ] - ) > 0; + ) > 0; } - public function checkPassword() { - $newPassword = $this->processor->getProperty('newpassword',null); + public function checkPassword() + { + $newPassword = $this->processor->getProperty('newpassword', null); $id = $this->processor->getProperty('id'); - $passwordGenerationMethod = $this->processor->getProperty('passwordgenmethod','g'); + $passwordGenerationMethod = $this->processor->getProperty('passwordgenmethod', 'g'); if ($passwordGenerationMethod !== 'user_email_specify' && ($newPassword !== null && $newPassword != 'false' || empty($id))) { - $passwordNotifyMethod = $this->processor->getProperty('passwordnotifymethod',null); + $passwordNotifyMethod = $this->processor->getProperty('passwordnotifymethod', null); if (empty($passwordNotifyMethod)) { - $this->processor->addFieldError('password_notify_method',$this->modx->lexicon('user_err_not_specified_notification_method')); + $this->processor->addFieldError('password_notify_method', $this->modx->lexicon('user_err_not_specified_notification_method')); } - if ($passwordGenerationMethod == 'g') { $autoPassword = $this->user->generatePassword(); $this->user->set('password', $autoPassword); - $this->processor->newPassword= $autoPassword; + $this->processor->newPassword = $autoPassword; } else { $specifiedPassword = $this->processor->getProperty('specifiedpassword'); $confirmPassword = $this->processor->getProperty('confirmpassword'); if (empty($specifiedPassword)) { - $this->processor->addFieldError('specifiedpassword',$this->modx->lexicon('user_err_not_specified_password')); + $this->processor->addFieldError('specifiedpassword', $this->modx->lexicon('user_err_not_specified_password')); } elseif ($specifiedPassword != $confirmPassword) { - $this->processor->addFieldError('confirmpassword',$this->modx->lexicon('user_err_password_no_match')); + $this->processor->addFieldError('confirmpassword', $this->modx->lexicon('user_err_password_no_match')); } elseif (strlen($specifiedPassword) < $this->modx->getOption('password_min_length', null, 8, true)) { - $this->processor->addFieldError('specifiedpassword',$this->modx->lexicon('user_err_password_too_short')); + $this->processor->addFieldError('specifiedpassword', $this->modx->lexicon('user_err_password_too_short')); } elseif (!preg_match('/^[^\'\x3c\x3e\(\);\x22\x7b\x7d\x2f\x5c]+$/', $specifiedPassword)) { $this->processor->addFieldError('specifiedpassword', $this->modx->lexicon('user_err_password_invalid')); } else { - $this->user->set('password',$specifiedPassword); + $this->user->set('password', $specifiedPassword); $this->processor->newPassword = $specifiedPassword; } } @@ -108,88 +114,93 @@ public function checkPassword() { return $this->processor->newPassword; } - public function checkEmail() { + public function checkEmail() + { $email = $this->processor->getProperty('email'); if (empty($email)) { - $this->processor->addFieldError('email',$this->modx->lexicon('user_err_not_specified_email')); + $this->processor->addFieldError('email', $this->modx->lexicon('user_err_not_specified_email')); } - if (!$this->modx->getOption('allow_multiple_emails',null,true)) { + if (!$this->modx->getOption('allow_multiple_emails', null, true)) { /** @var modUserProfile $emailExists */ $emailExists = $this->modx->getObject(modUserProfile::class, ['email' => $email]); if ($emailExists) { if ($emailExists->get('internalKey') != $this->processor->getProperty('id')) { - $this->processor->addFieldError('email',$this->modx->lexicon('user_err_already_exists_email')); + $this->processor->addFieldError('email', $this->modx->lexicon('user_err_already_exists_email')); } } } return $email; } - public function checkPhone() { + public function checkPhone() + { $phone = $this->processor->getProperty('phone'); if (!empty($phone)) { - if ($this->modx->getOption('clean_phone_number',null,false)) { - $phone = str_replace(' ','',$phone); - $phone = str_replace('-','',$phone); - $phone = str_replace('(','',$phone); - $phone = str_replace(')','',$phone); - $phone = str_replace('+','',$phone); - $this->processor->setProperty('phone',$phone); - $this->profile->set('phone',$phone); + if ($this->modx->getOption('clean_phone_number', null, false)) { + $phone = str_replace(' ', '', $phone); + $phone = str_replace('-', '', $phone); + $phone = str_replace('(', '', $phone); + $phone = str_replace(')', '', $phone); + $phone = str_replace('+', '', $phone); + $this->processor->setProperty('phone', $phone); + $this->profile->set('phone', $phone); } } } - public function checkCellPhone() { + public function checkCellPhone() + { $phone = $this->processor->getProperty('mobilephone'); if (!empty($phone)) { - if ($this->modx->getOption('clean_phone_number',null,false)) { - $phone = str_replace(' ','',$phone); - $phone = str_replace('-','',$phone); - $phone = str_replace('(','',$phone); - $phone = str_replace(')','',$phone); - $phone = str_replace('+','',$phone); - $this->processor->setProperty('mobilephone',$phone); - $this->profile->set('mobilephone',$phone); + if ($this->modx->getOption('clean_phone_number', null, false)) { + $phone = str_replace(' ', '', $phone); + $phone = str_replace('-', '', $phone); + $phone = str_replace('(', '', $phone); + $phone = str_replace(')', '', $phone); + $phone = str_replace('+', '', $phone); + $this->processor->setProperty('mobilephone', $phone); + $this->profile->set('mobilephone', $phone); } } } - public function checkBirthDate() { + public function checkBirthDate() + { $birthDate = $this->processor->getProperty('dob'); if (!empty($birthDate)) { - $birthDate = strtotime($birthDate); - if (false === $birthDate) { - $this->processor->addFieldError('dob',$this->modx->lexicon('user_err_not_specified_dob')); + $date = \DateTimeImmutable::createFromFormat($this->modx->getOption('manager_date_format', null, 'Y-m-d', true), $birthDate); + if ($date === false) { + $this->processor->addFieldError('dob', $this->modx->lexicon('user_err_not_specified_dob')); } - $this->processor->setProperty('dob',$birthDate); - $this->profile->set('dob',$birthDate); + $birthDate = $date->getTimestamp(); + $this->processor->setProperty('dob', $birthDate); + $this->profile->set('dob', $birthDate); } } - public function checkBlocked() { + public function checkBlocked() + { /* blocked until */ $blockedUntil = $this->processor->getProperty('blockeduntil'); if (!empty($blockedUntil)) { - $blockedUntil = str_replace('-','/',$blockedUntil); + $blockedUntil = str_replace('-', '/', $blockedUntil); if (!$blockedUntil = strtotime($blockedUntil)) { - $this->processor->addFieldError('blockeduntil',$this->modx->lexicon('user_err_not_specified_blockeduntil')); + $this->processor->addFieldError('blockeduntil', $this->modx->lexicon('user_err_not_specified_blockeduntil')); } - $this->processor->setProperty('blockeduntil',$blockedUntil); - $this->profile->set('blockeduntil',$blockedUntil); + $this->processor->setProperty('blockeduntil', $blockedUntil); + $this->profile->set('blockeduntil', $blockedUntil); } /* blocked after */ $blockedAfter = $this->processor->getProperty('blockedafter'); if (!empty($blockedAfter)) { - $blockedAfter = str_replace('-','/',$blockedAfter); + $blockedAfter = str_replace('-', '/', $blockedAfter); if (!$blockedAfter = strtotime($blockedAfter)) { - $this->processor->addFieldError('blockedafter',$this->modx->lexicon('user_err_not_specified_blockedafter')); + $this->processor->addFieldError('blockedafter', $this->modx->lexicon('user_err_not_specified_blockedafter')); } - $this->processor->setProperty('blockedafter',$blockedAfter); - $this->profile->set('blockedafter',$blockedAfter); + $this->processor->setProperty('blockedafter', $blockedAfter); + $this->profile->set('blockedafter', $blockedAfter); } } - }