Automattic /
jetpack
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 Jetpack_Protect_Module 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 Jetpack_Protect_Module, and based on these observations, apply Extract Interface, too.
| 1 | <?php |
||
| 17 | class Jetpack_Protect_Module { |
||
| 18 | |||
| 19 | private static $__instance = null; |
||
| 20 | public $api_key; |
||
| 21 | public $api_key_error; |
||
| 22 | public $whitelist; |
||
| 23 | public $whitelist_error; |
||
| 24 | public $whitelist_saved; |
||
| 25 | private $user_ip; |
||
| 26 | private $local_host; |
||
| 27 | private $api_endpoint; |
||
| 28 | public $last_request; |
||
| 29 | public $last_response_raw; |
||
| 30 | public $last_response; |
||
| 31 | private $block_login_with_math; |
||
| 32 | |||
| 33 | /** |
||
| 34 | * Singleton implementation |
||
| 35 | * |
||
| 36 | * @return object |
||
| 37 | */ |
||
| 38 | public static function instance() { |
||
| 45 | |||
| 46 | /** |
||
| 47 | * Registers actions |
||
| 48 | */ |
||
| 49 | private function __construct() { |
||
| 50 | add_action( 'jetpack_activate_module_protect', array ( $this, 'on_activation' ) ); |
||
| 51 | add_action( 'jetpack_deactivate_module_protect', array ( $this, 'on_deactivation' ) ); |
||
| 52 | add_action( 'jetpack_modules_loaded', array ( $this, 'modules_loaded' ) ); |
||
| 53 | add_action( 'login_init', array ( $this, 'check_use_math' ) ); |
||
| 54 | add_filter( 'authenticate', array ( $this, 'check_preauth' ), 10, 3 ); |
||
| 55 | add_action( 'wp_login', array ( $this, 'log_successful_login' ), 10, 2 ); |
||
| 56 | add_action( 'wp_login_failed', array ( $this, 'log_failed_attempt' ) ); |
||
| 57 | add_action( 'admin_init', array ( $this, 'maybe_update_headers' ) ); |
||
| 58 | add_action( 'admin_init', array ( $this, 'maybe_display_security_warning' ) ); |
||
| 59 | |||
| 60 | // This is a backup in case $pagenow fails for some reason |
||
| 61 | add_action( 'login_head', array ( $this, 'check_login_ability' ) ); |
||
| 62 | |||
| 63 | // Runs a script every day to clean up expired transients so they don't |
||
| 64 | // clog up our users' databases |
||
| 65 | require_once( JETPACK__PLUGIN_DIR . '/modules/protect/transient-cleanup.php' ); |
||
| 66 | } |
||
| 67 | |||
| 68 | /** |
||
| 69 | * On module activation, try to get an api key |
||
| 70 | */ |
||
| 71 | public function on_activation() { |
||
| 81 | |||
| 82 | /** |
||
| 83 | * On module deactivation, unset protect_active |
||
| 84 | */ |
||
| 85 | public function on_deactivation() { |
||
| 90 | |||
| 91 | public function maybe_get_protect_key() { |
||
| 92 | if ( get_site_option( 'jetpack_protect_activating', false ) && ! get_site_option( 'jetpack_protect_key', false ) ) { |
||
| 93 | $key = $this->get_protect_key(); |
||
| 94 | delete_site_option( 'jetpack_protect_activating' ); |
||
| 95 | return $key; |
||
| 96 | } |
||
| 97 | |||
| 98 | return get_site_option( 'jetpack_protect_key' ); |
||
| 99 | } |
||
| 100 | |||
| 101 | /** |
||
| 102 | * Sends a "check_key" API call once a day. This call allows us to track IP-related |
||
| 103 | * headers for this server via the Protect API, in order to better identify the source |
||
| 104 | * IP for login attempts |
||
| 105 | */ |
||
| 106 | public function maybe_update_headers( $force = false ) { |
||
| 129 | |||
| 130 | public function maybe_display_security_warning() { |
||
| 141 | |||
| 142 | public function prepare_jetpack_protect_multisite_notice() { |
||
| 146 | |||
| 147 | public function admin_banner_styles() { |
||
| 155 | |||
| 156 | public function admin_jetpack_manage_notice() { |
||
| 185 | |||
| 186 | /** |
||
| 187 | * Request an api key from wordpress.com |
||
| 188 | * |
||
| 189 | * @return bool | string |
||
| 190 | */ |
||
| 191 | public function get_protect_key() { |
||
| 263 | |||
| 264 | /** |
||
| 265 | * Called via WP action wp_login_failed to log failed attempt with the api |
||
| 266 | * |
||
| 267 | * Fires custom, plugable action jpp_log_failed_attempt with the IP |
||
| 268 | * |
||
| 269 | * @return void |
||
| 270 | */ |
||
| 271 | function log_failed_attempt() { |
||
| 298 | |||
| 299 | /** |
||
| 300 | * Set up the Protect configuration page |
||
| 301 | */ |
||
| 302 | public function modules_loaded() { |
||
| 308 | |||
| 309 | /** |
||
| 310 | * Logs a successful login back to our servers, this allows us to make sure we're not blocking |
||
| 311 | * a busy IP that has a lot of good logins along with some forgotten passwords. Also saves current user's ip |
||
| 312 | * to the ip address whitelist |
||
| 313 | */ |
||
| 314 | public function log_successful_login( $user_login, $user ) { |
||
| 317 | |||
| 318 | |||
| 319 | /** |
||
| 320 | * Checks for loginability BEFORE authentication so that bots don't get to go around the log in form. |
||
| 321 | * |
||
| 322 | * If we are using our math fallback, authenticate via math-fallback.php |
||
| 323 | * |
||
| 324 | * @param string $user |
||
| 325 | * @param string $username |
||
| 326 | * @param string $password |
||
| 327 | * |
||
| 328 | * @return string $user |
||
| 329 | */ |
||
| 330 | function check_preauth( $user = 'Not Used By Protect', $username = 'Not Used By Protect', $password = 'Not Used By Protect' ) { |
||
| 345 | |||
| 346 | /** |
||
| 347 | * Get all IP headers so that we can process on our server... |
||
| 348 | * |
||
| 349 | * @return string |
||
| 350 | */ |
||
| 351 | function get_headers() { |
||
| 380 | |||
| 381 | /* |
||
| 382 | * Checks if the IP address has been whitelisted |
||
| 383 | * |
||
| 384 | * @param string $ip |
||
| 385 | * |
||
| 386 | * @return bool |
||
| 387 | */ |
||
| 388 | function ip_is_whitelisted( $ip ) { |
||
| 417 | |||
| 418 | /** |
||
| 419 | * Checks the status for a given IP. API results are cached as transients |
||
| 420 | * |
||
| 421 | * @param bool $preauth Whether or not we are checking prior to authorization |
||
| 422 | * |
||
| 423 | * @return bool Either returns true, fires $this->kill_login, or includes a math fallback and returns false |
||
| 424 | */ |
||
| 425 | function check_login_ability( $preauth = false ) { |
||
| 426 | $ip = jetpack_protect_get_ip(); |
||
| 427 | |||
| 428 | // Server is misconfigured and we can't get an IP |
||
| 429 | if ( ! $ip && class_exists( 'Jetpack' ) ) { |
||
| 430 | Jetpack::deactivate_module( 'protect' ); |
||
| 431 | ob_start(); |
||
| 432 | Jetpack::state( 'message', 'protect_misconfigured_ip' ); |
||
| 433 | ob_end_clean(); |
||
| 434 | return true; |
||
| 435 | } |
||
| 436 | |||
| 437 | /** |
||
| 438 | * Short-circuit check_login_ability. |
||
| 439 | * |
||
| 440 | * If there is an alternate way to validate the current IP such as |
||
| 441 | * a hard-coded list of IP addresses, we can short-circuit the rest |
||
| 442 | * of the login ability checks and return true here. |
||
| 443 | * |
||
| 444 | * @module protect |
||
| 445 | * |
||
| 446 | * @since 4.4.0 |
||
| 447 | * |
||
| 448 | * @param bool false Should we allow all logins for the current ip? Default: false |
||
| 449 | */ |
||
| 450 | if ( apply_filters( 'jpp_allow_login', false, $ip ) ) { |
||
| 451 | return true; |
||
| 452 | } |
||
| 453 | |||
| 454 | $headers = $this->get_headers(); |
||
| 455 | $header_hash = md5( json_encode( $headers ) ); |
||
| 456 | $transient_name = 'jpp_li_' . $header_hash; |
||
| 457 | $transient_value = $this->get_transient( $transient_name ); |
||
| 458 | |||
| 459 | if ( jetpack_protect_ip_is_private( $ip ) ) { |
||
| 460 | return true; |
||
| 461 | } |
||
| 462 | |||
| 463 | if ( $this->ip_is_whitelisted( $ip ) ) { |
||
| 464 | return true; |
||
| 465 | } |
||
| 466 | |||
| 467 | // Check out our transients |
||
| 468 | if ( isset( $transient_value ) && 'ok' == $transient_value['status'] ) { |
||
| 469 | return true; |
||
| 470 | } |
||
| 471 | |||
| 472 | if ( isset( $transient_value ) && 'blocked' == $transient_value['status'] ) { |
||
| 473 | $this->block_with_math(); |
||
| 474 | } |
||
| 475 | |||
| 476 | if ( isset( $transient_value ) && 'blocked-hard' == $transient_value['status'] ) { |
||
| 477 | $this->kill_login(); |
||
| 478 | } |
||
| 479 | |||
| 480 | // If we've reached this point, this means that the IP isn't cached. |
||
| 481 | // Now we check with the Protect API to see if we should allow login |
||
| 482 | $response = $this->protect_call( $action = 'check_ip' ); |
||
| 483 | |||
| 484 | if ( isset( $response['math'] ) && ! function_exists( 'brute_math_authenticate' ) ) { |
||
| 485 | include_once dirname( __FILE__ ) . '/protect/math-fallback.php'; |
||
| 486 | new Jetpack_Protect_Math_Authenticate; |
||
| 487 | |||
| 488 | return false; |
||
| 489 | } |
||
| 490 | |||
| 491 | if ( 'blocked' == $response['status'] ) { |
||
| 492 | $this->block_with_math(); |
||
| 493 | } |
||
| 494 | |||
| 495 | if ( 'blocked-hard' == $response['status'] ) { |
||
| 496 | $this->kill_login(); |
||
| 497 | } |
||
| 498 | |||
| 499 | return true; |
||
| 500 | } |
||
| 501 | |||
| 502 | function block_with_math() { |
||
| 535 | |||
| 536 | /* |
||
| 537 | * Kill a login attempt |
||
| 538 | */ |
||
| 539 | function kill_login() { |
||
| 565 | |||
| 566 | /* |
||
| 567 | * Checks if the protect API call has failed, and if so initiates the math captcha fallback. |
||
| 568 | */ |
||
| 569 | public function check_use_math() { |
||
| 576 | |||
| 577 | /** |
||
| 578 | * Get or delete API key |
||
| 579 | */ |
||
| 580 | public function configuration_load() { |
||
| 602 | |||
| 603 | public function configuration_head() { |
||
| 606 | |||
| 607 | /** |
||
| 608 | * Prints the configuration screen |
||
| 609 | */ |
||
| 610 | public function configuration_screen() { |
||
| 613 | |||
| 614 | /** |
||
| 615 | * If we're in a multisite network, return the blog ID of the primary blog |
||
| 616 | * |
||
| 617 | * @return int |
||
| 618 | */ |
||
| 619 | public function get_main_blog_id() { |
||
| 629 | |||
| 630 | /** |
||
| 631 | * Get jetpack blog id, or the jetpack blog id of the main blog in the main network |
||
| 632 | * |
||
| 633 | * @return int |
||
| 634 | */ |
||
| 635 | public function get_main_blog_jetpack_id() { |
||
| 646 | |||
| 647 | public function check_api_key() { |
||
| 669 | |||
| 670 | /** |
||
| 671 | * Calls over to the api using wp_remote_post |
||
| 672 | * |
||
| 673 | * @param string $action 'check_ip', 'check_key', or 'failed_attempt' |
||
| 674 | * @param array $request Any custom data to post to the api |
||
| 675 | * |
||
| 676 | * @return array |
||
| 677 | */ |
||
| 678 | function protect_call( $action = 'check_ip', $request = array () ) { |
||
| 750 | |||
| 751 | |||
| 752 | /** |
||
| 753 | * Wrapper for WordPress set_transient function, our version sets |
||
| 754 | * the transient on the main site in the network if this is a multisite network |
||
| 755 | * |
||
| 756 | * We do it this way (instead of set_site_transient) because of an issue where |
||
| 757 | * sitewide transients are always autoloaded |
||
| 758 | * https://core.trac.wordpress.org/ticket/22846 |
||
| 759 | * |
||
| 760 | * @param string $transient Transient name. Expected to not be SQL-escaped. Must be |
||
| 761 | * 45 characters or fewer in length. |
||
| 762 | * @param mixed $value Transient value. Must be serializable if non-scalar. |
||
| 763 | * Expected to not be SQL-escaped. |
||
| 764 | * @param int $expiration Optional. Time until expiration in seconds. Default 0. |
||
| 765 | * |
||
| 766 | * @return bool False if value was not set and true if value was set. |
||
| 767 | */ |
||
| 768 | function set_transient( $transient, $value, $expiration ) { |
||
| 779 | |||
| 780 | /** |
||
| 781 | * Wrapper for WordPress delete_transient function, our version deletes |
||
| 782 | * the transient on the main site in the network if this is a multisite network |
||
| 783 | * |
||
| 784 | * @param string $transient Transient name. Expected to not be SQL-escaped. |
||
| 785 | * |
||
| 786 | * @return bool true if successful, false otherwise |
||
| 787 | */ |
||
| 788 | View Code Duplication | function delete_transient( $transient ) { |
|
| 799 | |||
| 800 | /** |
||
| 801 | * Wrapper for WordPress get_transient function, our version gets |
||
| 802 | * the transient on the main site in the network if this is a multisite network |
||
| 803 | * |
||
| 804 | * @param string $transient Transient name. Expected to not be SQL-escaped. |
||
| 805 | * |
||
| 806 | * @return mixed Value of transient. |
||
| 807 | */ |
||
| 808 | View Code Duplication | function get_transient( $transient ) { |
|
| 819 | |||
| 820 | function get_api_host() { |
||
| 830 | |||
| 831 | function get_local_host() { |
||
| 857 | |||
| 858 | } |
||
| 859 | |||
| 865 |
Loading history...
Adding an explicit array definition is generally preferable to implicit array definition as it guarantees a stable state of the code.
Let’s take a look at an example:
As you can see in this example, the array
$myArrayis initialized the first time when the foreach loop is entered. You can also see that the value of thebarkey is only written conditionally; thus, its value might result from a previous iteration.This might or might not be intended. To make your intention clear, your code more readible and to avoid accidental bugs, we recommend to add an explicit initialization $myArray = array() either outside or inside the foreach loop.