panique /
huge
Complex classes like PasswordResetModel often do a lot of different things. To break such a class down, we need to identify a cohesive component within that class. A common approach to find such a component is to look for fields/methods that share the same prefixes, or suffixes. You can also have a look at the cohesion graph to spot any un-connected, or weakly-connected components.
Once you have determined the fields that belong together, you can apply the Extract Class refactoring. If the component makes sense as a sub-class, Extract Subclass is also a candidate, and is often faster.
While breaking up the class, it is a good idea to analyze how other classes use PasswordResetModel, and based on these observations, apply Extract Interface, too.
| 1 | <?php |
||
| 8 | class PasswordResetModel |
||
|
|
|||
| 9 | { |
||
| 10 | /** |
||
| 11 | * Perform the necessary actions to send a password reset mail |
||
| 12 | * |
||
| 13 | * @param $user_name_or_email string Username or user's email |
||
| 14 | * @param $captcha string Captcha string |
||
| 15 | * |
||
| 16 | * @return bool success status |
||
| 17 | */ |
||
| 18 | public static function requestPasswordReset($user_name_or_email, $captcha) |
||
| 19 | { |
||
| 20 | if (!CaptchaModel::checkCaptcha($captcha)) { |
||
| 21 | Session::add('feedback_negative', Text::get('FEEDBACK_CAPTCHA_WRONG')); |
||
| 22 | return false; |
||
| 23 | } |
||
| 24 | |||
| 25 | if (empty($user_name_or_email)) { |
||
| 26 | Session::add('feedback_negative', Text::get('FEEDBACK_USERNAME_EMAIL_FIELD_EMPTY')); |
||
| 27 | return false; |
||
| 28 | } |
||
| 29 | |||
| 30 | // check if that username exists |
||
| 31 | $result = UserModel::getUserDataByUserNameOrEmail($user_name_or_email); |
||
| 32 | if (!$result) { |
||
| 33 | Session::add('feedback_negative', Text::get('FEEDBACK_USER_DOES_NOT_EXIST')); |
||
| 34 | return false; |
||
| 35 | } |
||
| 36 | |||
| 37 | // generate integer-timestamp (to see when exactly the user (or an attacker) requested the password reset mail) |
||
| 38 | // generate random hash for email password reset verification (40 char string) |
||
| 39 | $temporary_timestamp = time(); |
||
| 40 | $user_password_reset_hash = sha1(uniqid(mt_rand(), true)); |
||
| 41 | |||
| 42 | // set token (= a random hash string and a timestamp) into database ... |
||
| 43 | $token_set = self::setPasswordResetDatabaseToken($result->user_name, $user_password_reset_hash, $temporary_timestamp); |
||
| 44 | if (!$token_set) { |
||
| 45 | return false; |
||
| 46 | } |
||
| 47 | |||
| 48 | // ... and send a mail to the user, containing a link with username and token hash string |
||
| 49 | $mail_sent = self::sendPasswordResetMail($result->user_name, $user_password_reset_hash, $result->user_email); |
||
| 50 | if ($mail_sent) { |
||
| 51 | return true; |
||
| 52 | } |
||
| 53 | |||
| 54 | // default return |
||
| 55 | return false; |
||
| 56 | } |
||
| 57 | |||
| 58 | /** |
||
| 59 | * Set password reset token in database (for DEFAULT user accounts) |
||
| 60 | * |
||
| 61 | * @param string $user_name username |
||
| 62 | * @param string $user_password_reset_hash password reset hash |
||
| 63 | * @param int $temporary_timestamp timestamp |
||
| 64 | * |
||
| 65 | * @return bool success status |
||
| 66 | */ |
||
| 67 | public static function setPasswordResetDatabaseToken($user_name, $user_password_reset_hash, $temporary_timestamp) |
||
| 89 | |||
| 90 | /** |
||
| 91 | * Send the password reset mail |
||
| 92 | * |
||
| 93 | * @param string $user_name username |
||
| 94 | * @param string $user_password_reset_hash password reset hash |
||
| 95 | * @param string $user_email user email |
||
| 96 | * |
||
| 97 | * @return bool success status |
||
| 98 | */ |
||
| 99 | public static function sendPasswordResetMail($user_name, $user_password_reset_hash, $user_email) |
||
| 119 | |||
| 120 | /** |
||
| 121 | * Verifies the password reset request via the verification hash token (that's only valid for one hour) |
||
| 122 | * @param string $user_name Username |
||
| 123 | * @param string $verification_code Hash token |
||
| 124 | * @return bool Success status |
||
| 125 | */ |
||
| 126 | public static function verifyPasswordReset($user_name, $verification_code) |
||
| 178 | |||
| 179 | /** |
||
| 180 | * Marks existing password reset request as expired to prevent prevent guessing hash codes |
||
| 181 | * @param string $user_name Username |
||
| 182 | * @return bool Success status |
||
| 183 | */ |
||
| 184 | private static function expirePasswordReset($user_name) { |
||
| 209 | |||
| 210 | /** |
||
| 211 | * Writes the new password to the database |
||
| 212 | * |
||
| 213 | * @param string $user_name username |
||
| 214 | * @param string $user_password_hash |
||
| 215 | * @param string $user_password_reset_hash |
||
| 216 | * |
||
| 217 | * @return bool |
||
| 218 | */ |
||
| 219 | public static function saveNewUserPassword($user_name, $user_password_hash, $user_password_reset_hash) |
||
| 236 | |||
| 237 | /** |
||
| 238 | * Set the new password (for DEFAULT user, FACEBOOK-users don't have a password) |
||
| 239 | * Please note: At this point the user has already pre-verified via verifyPasswordReset() (within one hour), |
||
| 240 | * so we don't need to check again for the 60min-limit here. In this method we authenticate |
||
| 241 | * via username & password-reset-hash from (hidden) form fields. |
||
| 242 | * |
||
| 243 | * @param string $user_name |
||
| 244 | * @param string $user_password_reset_hash |
||
| 245 | * @param string $user_password_new |
||
| 246 | * @param string $user_password_repeat |
||
| 247 | * |
||
| 248 | * @return bool success state of the password reset |
||
| 249 | */ |
||
| 250 | public static function setNewPassword($user_name, $user_password_reset_hash, $user_password_new, $user_password_repeat) |
||
| 269 | |||
| 270 | /** |
||
| 271 | * Validate the password submission |
||
| 272 | * |
||
| 273 | * @param $user_name |
||
| 274 | * @param $user_password_reset_hash |
||
| 275 | * @param $user_password_new |
||
| 276 | * @param $user_password_repeat |
||
| 277 | * |
||
| 278 | * @return bool |
||
| 279 | */ |
||
| 280 | public static function validateResetPassword($user_name, $user_password_reset_hash, $user_password_new, $user_password_repeat) |
||
| 301 | |||
| 302 | |||
| 303 | /** |
||
| 304 | * Writes the new password to the database |
||
| 305 | * |
||
| 306 | * @param string $user_name |
||
| 307 | * @param string $user_password_hash |
||
| 308 | * |
||
| 309 | * @return bool |
||
| 310 | */ |
||
| 311 | public static function saveChangedPassword($user_name, $user_password_hash) |
||
| 327 | |||
| 328 | |||
| 329 | /** |
||
| 330 | * Validates fields, hashes new password, saves new password |
||
| 331 | * |
||
| 332 | * @param string $user_name |
||
| 333 | * @param string $user_password_current |
||
| 334 | * @param string $user_password_new |
||
| 335 | * @param string $user_password_repeat |
||
| 336 | * |
||
| 337 | * @return bool |
||
| 338 | */ |
||
| 339 | public static function changePassword($user_name, $user_password_current, $user_password_new, $user_password_repeat) |
||
| 358 | |||
| 359 | |||
| 360 | /** |
||
| 361 | * Validates current and new passwords |
||
| 362 | * |
||
| 363 | * @param string $user_name |
||
| 364 | * @param string $user_password_current |
||
| 365 | * @param string $user_password_new |
||
| 366 | * @param string $user_password_repeat |
||
| 367 | * |
||
| 368 | * @return bool |
||
| 369 | */ |
||
| 370 | public static function validatePasswordChange($user_name, $user_password_current, $user_password_new, $user_password_repeat) |
||
| 408 | } |
||
| 409 |
You can fix this by adding a namespace to your class:
When choosing a vendor namespace, try to pick something that is not too generic to avoid conflicts with other libraries.