ci-bonfire /
Sprint
Duplicate code is one of the most pungent code smells. A rule that is often used is to re-structure code once it is duplicated in three or more places.
Common duplication problems, and corresponding solutions are:
Complex classes like LocalAuthentication 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 LocalAuthentication, and based on these observations, apply Extract Interface, too.
| 1 | <?php namespace Myth\Auth; |
||
| 53 | class LocalAuthentication implements AuthenticateInterface { |
||
| 54 | |||
| 55 | protected $ci; |
||
| 56 | |||
| 57 | protected $user = null; |
||
| 58 | |||
| 59 | public $user_model = null; |
||
| 60 | |||
| 61 | public $error = null; |
||
| 62 | |||
| 63 | //-------------------------------------------------------------------- |
||
| 64 | |||
| 65 | public function __construct( $ci=null ) |
||
| 91 | |||
| 92 | //-------------------------------------------------------------------- |
||
| 93 | |||
| 94 | /** |
||
| 95 | * Attempt to log a user into the system. |
||
| 96 | * |
||
| 97 | * $credentials is an array of key/value pairs needed to log the user in. |
||
| 98 | * This is often email/password, or username/password. |
||
| 99 | * |
||
| 100 | * @param array $credentials |
||
| 101 | * @param bool $remember |
||
| 102 | * @return bool|mixed |
||
| 103 | */ |
||
| 104 | public function login($credentials, $remember=false) |
||
| 144 | |||
| 145 | //-------------------------------------------------------------------- |
||
| 146 | |||
| 147 | /** |
||
| 148 | * Validates user login information without logging them in. |
||
| 149 | * |
||
| 150 | * $credentials is an array of key/value pairs needed to log the user in. |
||
| 151 | * This is often email/password, or username/password. |
||
| 152 | * |
||
| 153 | * @param $credentials |
||
| 154 | * @param bool $return_user |
||
| 155 | * @return mixed |
||
| 156 | */ |
||
| 157 | public function validate($credentials, $return_user=false) |
||
| 158 | { |
||
| 159 | // Get ip address |
||
| 160 | $ip_address = $this->ci->input->ip_address(); |
||
| 161 | |||
| 162 | // We do not want to force case-sensitivity on things |
||
| 163 | // like username and email for usability sake. |
||
| 164 | if (! empty($credentials['email'])) |
||
| 165 | { |
||
| 166 | $credentials['email'] = strtolower($credentials['email']); |
||
| 167 | } |
||
| 168 | |||
| 169 | // Can't validate without a password. |
||
| 170 | if (empty($credentials['password']) || count($credentials) < 2) |
||
| 171 | { |
||
| 172 | $this->ci->login_model->recordLoginAttempt($ip_address); |
||
| 173 | return null; |
||
| 174 | } |
||
| 175 | |||
| 176 | $password = $credentials['password']; |
||
| 177 | unset($credentials['password']); |
||
| 178 | |||
| 179 | // We should only be allowed 1 single other credential to |
||
| 180 | // test against. |
||
| 181 | if (count($credentials) > 1) |
||
| 182 | { |
||
| 183 | $this->error = lang('auth.too_many_credentials'); |
||
| 184 | $this->ci->login_model->recordLoginAttempt($ip_address); |
||
| 185 | return false; |
||
| 186 | } |
||
| 187 | |||
| 188 | // Ensure that the fields are allowed validation fields |
||
| 189 | if (! in_array(key($credentials), config_item('auth.valid_fields')) ) |
||
| 190 | { |
||
| 191 | $this->error = lang('auth.invalid_credentials'); |
||
| 192 | $this->ci->login_model->recordLoginAttempt($ip_address); |
||
| 193 | return false; |
||
| 194 | } |
||
| 195 | |||
| 196 | // Can we find a user with those credentials? |
||
| 197 | $user = $this->user_model->as_array() |
||
| 198 | ->where($credentials) |
||
| 199 | ->first(); |
||
| 200 | |||
| 201 | if (! $user) |
||
| 202 | { |
||
| 203 | $this->error = lang('auth.invalid_user'); |
||
| 204 | $this->ci->login_model->recordLoginAttempt($ip_address); |
||
| 205 | return false; |
||
| 206 | } |
||
| 207 | |||
| 208 | // Now, try matching the passwords. |
||
| 209 | $result = password_verify($password, $user['password_hash']); |
||
| 210 | |||
| 211 | if (! $result) |
||
| 212 | { |
||
| 213 | $this->error = lang('auth.invalid_password'); |
||
| 214 | $this->ci->login_model->recordLoginAttempt($ip_address, $user['id']); |
||
| 215 | return false; |
||
| 216 | } |
||
| 217 | |||
| 218 | // Check to see if the password needs to be rehashed. |
||
| 219 | // This would be due to the hash algorithm or hash |
||
| 220 | // cost changing since the last time that a user |
||
| 221 | // logged in. |
||
| 222 | if (password_needs_rehash($user['password_hash'], PASSWORD_DEFAULT, ['cost' => config_item('auth.hash_cost')] )) |
||
| 223 | { |
||
| 224 | $new_hash = Password::hashPassword($password); |
||
| 225 | $this->user_model->skip_validation() |
||
| 226 | ->update($user['id'], ['password_hash' => $new_hash]); |
||
| 227 | unset($new_hash); |
||
| 228 | } |
||
| 229 | |||
| 230 | // Is the user active? |
||
| 231 | if (! $user['active']) |
||
| 232 | { |
||
| 233 | $this->error = lang('auth.inactive_account'); |
||
| 234 | return false; |
||
| 235 | } |
||
| 236 | |||
| 237 | return $return_user ? $user : true; |
||
| 238 | } |
||
| 239 | |||
| 240 | //-------------------------------------------------------------------- |
||
| 241 | |||
| 242 | /** |
||
| 243 | * Logs a user out and removes all session information. |
||
| 244 | * |
||
| 245 | * @return mixed |
||
| 246 | */ |
||
| 247 | public function logout() |
||
| 277 | |||
| 278 | //-------------------------------------------------------------------- |
||
| 279 | |||
| 280 | /** |
||
| 281 | * Checks whether a user is logged in or not. |
||
| 282 | * |
||
| 283 | * @return bool |
||
| 284 | */ |
||
| 285 | public function isLoggedIn() |
||
| 314 | |||
| 315 | //-------------------------------------------------------------------- |
||
| 316 | |||
| 317 | /** |
||
| 318 | * Attempts to log a user in based on the "remember me" cookie. |
||
| 319 | * |
||
| 320 | * @return bool |
||
| 321 | */ |
||
| 322 | public function viaRemember() |
||
| 359 | |||
| 360 | //-------------------------------------------------------------------- |
||
| 361 | |||
| 362 | /** |
||
| 363 | * Registers a new user and handles activation method. |
||
| 364 | * |
||
| 365 | * @param $user_data |
||
| 366 | * @return bool |
||
| 367 | */ |
||
| 368 | public function registerUser($user_data) |
||
| 404 | |||
| 405 | //-------------------------------------------------------------------- |
||
| 406 | |||
| 407 | /** |
||
| 408 | * Used to verify the user values and activate a user so they can |
||
| 409 | * visit the site. |
||
| 410 | * |
||
| 411 | * @param $data |
||
| 412 | * @return bool |
||
| 413 | */ |
||
| 414 | public function activateUser($data) |
||
| 440 | |||
| 441 | //-------------------------------------------------------------------- |
||
| 442 | |||
| 443 | /** |
||
| 444 | * Used to allow manual activation of a user with a known ID. |
||
| 445 | * |
||
| 446 | * @param $id |
||
| 447 | * @return bool |
||
| 448 | */ |
||
| 449 | public function activateUserById($id) |
||
| 461 | |||
| 462 | //-------------------------------------------------------------------- |
||
| 463 | |||
| 464 | /** |
||
| 465 | * Grabs the current user object. Returns NULL if nothing found. |
||
| 466 | * |
||
| 467 | * @return array|null |
||
| 468 | */ |
||
| 469 | public function user() |
||
| 473 | |||
| 474 | //-------------------------------------------------------------------- |
||
| 475 | |||
| 476 | /** |
||
| 477 | * A convenience method to grab the current user's ID. |
||
| 478 | * |
||
| 479 | * @return int|null |
||
| 480 | */ |
||
| 481 | public function id() |
||
| 490 | |||
| 491 | //-------------------------------------------------------------------- |
||
| 492 | |||
| 493 | /** |
||
| 494 | * Checks to see if the user is currently being throttled. |
||
| 495 | * |
||
| 496 | * - If they are NOT, will return FALSE. |
||
| 497 | * - If they ARE, will return the number of seconds until they can try again. |
||
| 498 | * |
||
| 499 | * @param $email |
||
| 500 | * @return mixed |
||
| 501 | */ |
||
| 502 | public function isThrottled($user) |
||
| 603 | |||
| 604 | //-------------------------------------------------------------------- |
||
| 605 | |||
| 606 | /** |
||
| 607 | * Sends a password reset link email to the user associated with |
||
| 608 | * the passed in $email. |
||
| 609 | * |
||
| 610 | * @param $email |
||
| 611 | * @return mixed |
||
| 612 | */ |
||
| 613 | public function remindUser($email) |
||
| 644 | |||
| 645 | //-------------------------------------------------------------------- |
||
| 646 | |||
| 647 | /** |
||
| 648 | * Validates the credentials provided and, if valid, resets the password. |
||
| 649 | * |
||
| 650 | * The $credentials array MUST contain a 'code' key with the string to |
||
| 651 | * hash and check against the reset_hash. |
||
| 652 | * |
||
| 653 | * @param $credentials |
||
| 654 | * @param $password |
||
| 655 | * @param $passConfirm |
||
| 656 | * @return mixed |
||
| 657 | */ |
||
| 658 | public function resetPassword($credentials, $password, $passConfirm) |
||
| 701 | |||
| 702 | //-------------------------------------------------------------------- |
||
| 703 | |||
| 704 | /** |
||
| 705 | * Provides a way for implementations to allow new statuses to be set |
||
| 706 | * on the user. The details will vary based upon implementation, but |
||
| 707 | * will often allow for banning or suspending users. |
||
| 708 | * |
||
| 709 | * @param $newStatus |
||
| 710 | * @param null $message |
||
| 711 | * @return mixed |
||
| 712 | */ |
||
| 713 | public function changeStatus($newStatus, $message=null) |
||
| 717 | |||
| 718 | //-------------------------------------------------------------------- |
||
| 719 | |||
| 720 | /** |
||
| 721 | * Allows the consuming application to pass in a reference to the |
||
| 722 | * model that should be used. |
||
| 723 | * |
||
| 724 | * The model MUST extend Myth\Models\CIDbModel. |
||
| 725 | * |
||
| 726 | * @param $model |
||
| 727 | * @param bool $allow_any_parent |
||
| 728 | * @return mixed |
||
| 729 | */ |
||
| 730 | public function useModel($model, $allow_any_parent=false) |
||
| 741 | |||
| 742 | //-------------------------------------------------------------------- |
||
| 743 | |||
| 744 | public function error() |
||
| 753 | |||
| 754 | //-------------------------------------------------------------------- |
||
| 755 | |||
| 756 | //-------------------------------------------------------------------- |
||
| 757 | // Login Records |
||
| 758 | //-------------------------------------------------------------------- |
||
| 759 | |||
| 760 | /** |
||
| 761 | * Purges all login attempt records from the database. |
||
| 762 | * |
||
| 763 | * @param $email |
||
| 764 | */ |
||
| 765 | View Code Duplication | public function purgeLoginAttempts($email) |
|
| 775 | |||
| 776 | //-------------------------------------------------------------------- |
||
| 777 | |||
| 778 | /** |
||
| 779 | * Purges all remember tokens for a single user. Effectively logs |
||
| 780 | * a user out of all devices. Intended to allow users to log themselves |
||
| 781 | * out of all devices as a security measure. |
||
| 782 | * |
||
| 783 | * @param $email |
||
| 784 | */ |
||
| 785 | View Code Duplication | public function purgeRememberTokens($email) |
|
| 795 | |||
| 796 | //-------------------------------------------------------------------- |
||
| 797 | |||
| 798 | //-------------------------------------------------------------------- |
||
| 799 | // Protected Methods |
||
| 800 | //-------------------------------------------------------------------- |
||
| 801 | |||
| 802 | /** |
||
| 803 | * Check if Allow Persistent Login Cookies is enable |
||
| 804 | * |
||
| 805 | * @param $user |
||
| 806 | */ |
||
| 807 | protected function rememberUser($user) |
||
| 817 | |||
| 818 | //-------------------------------------------------------------------- |
||
| 819 | |||
| 820 | /** |
||
| 821 | * Invalidates the current rememberme cookie/database entry, creates |
||
| 822 | * a new one, stores it and returns the new value. |
||
| 823 | * |
||
| 824 | * @param $user |
||
| 825 | * @param null $token |
||
| 826 | * @return mixed |
||
| 827 | */ |
||
| 828 | protected function refreshRememberCookie($user, $token=null) |
||
| 864 | |||
| 865 | //-------------------------------------------------------------------- |
||
| 866 | |||
| 867 | /** |
||
| 868 | * Deletes any current remember me cookies and database entries. |
||
| 869 | * |
||
| 870 | * @param $email |
||
| 871 | * @param $token |
||
| 872 | * @return string The new token (not the hash). |
||
| 873 | */ |
||
| 874 | protected function invalidateRememberCookie($email, $token) |
||
| 890 | |||
| 891 | //-------------------------------------------------------------------- |
||
| 892 | |||
| 893 | /** |
||
| 894 | * Handles the nitty gritty of actually logging our user into the system. |
||
| 895 | * Does NOT perform the authentication, just sets the system up so that |
||
| 896 | * it knows we're here. |
||
| 897 | * |
||
| 898 | * @param $user |
||
| 899 | */ |
||
| 900 | protected function loginUser($user) |
||
| 933 | |||
| 934 | //-------------------------------------------------------------------- |
||
| 935 | |||
| 936 | /** |
||
| 937 | * Sets the headers to ensure that pages are not cached when a user |
||
| 938 | * is logged in, helping to protect against logging out and then |
||
| 939 | * simply hitting the Back button on the browser and getting private |
||
| 940 | * information because the page was loaded from cache. |
||
| 941 | */ |
||
| 942 | protected function setHeaders() |
||
| 948 | |||
| 949 | //-------------------------------------------------------------------- |
||
| 950 | |||
| 951 | |||
| 952 | } |
||
| 953 |
Instead of super-globals, we recommend to explicitly inject the dependencies of your class. This makes your code less dependent on global state and it becomes generally more testable: