-
Notifications
You must be signed in to change notification settings - Fork 116
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Improve client IP resolution for Stream logs #1459
Changes from all commits
4d086cc
868957b
970121b
5578066
54150d6
2a00ddf
c637256
fb0f093
c10f996
d18e872
3d11efd
d9fa985
4afe6ec
f0a5e20
a136099
560baf1
94e17bd
9daaaa8
1b9068d
38683c1
f6c799b
85bdc4d
9bcc490
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -19,14 +19,6 @@ class Log { | |
*/ | ||
public $plugin; | ||
|
||
/** | ||
* Hold Current visitors IP Address. | ||
* | ||
* @var string | ||
*/ | ||
private $ip_address; | ||
|
||
|
||
/** | ||
* Previous Stream record ID, used for chaining same-session records | ||
* | ||
|
@@ -42,12 +34,6 @@ class Log { | |
public function __construct( $plugin ) { | ||
$this->plugin = $plugin; | ||
|
||
// Support proxy mode by checking the `X-Forwarded-For` header first. | ||
$ip_address = wp_stream_filter_input( INPUT_SERVER, 'HTTP_X_FORWARDED_FOR', FILTER_VALIDATE_IP ); | ||
$ip_address = $ip_address ? $ip_address : wp_stream_filter_input( INPUT_SERVER, 'REMOTE_ADDR', FILTER_VALIDATE_IP ); | ||
|
||
$this->ip_address = $ip_address; | ||
|
||
// Ensure function used in various methods is pre-loaded. | ||
if ( ! function_exists( 'is_plugin_active_for_network' ) ) { | ||
require_once ABSPATH . '/wp-admin/includes/plugin.php'; | ||
|
@@ -87,9 +73,11 @@ public function log( $connector, $message, $args, $object_id, $context, $action, | |
return false; | ||
} | ||
|
||
$ip_address = $this->plugin->get_client_ip_address(); | ||
|
||
$user = new \WP_User( $user_id ); | ||
|
||
if ( $this->is_record_excluded( $connector, $context, $action, $user ) ) { | ||
if ( $this->is_record_excluded( $connector, $context, $action, $user, $ip_address ) ) { | ||
return false; | ||
} | ||
|
||
|
@@ -140,7 +128,7 @@ function ( $var ) { | |
'connector' => (string) $connector, | ||
'context' => (string) $context, | ||
'action' => (string) $action, | ||
'ip' => (string) $this->ip_address, | ||
'ip' => (string) $ip_address, | ||
schlessera marked this conversation as resolved.
Show resolved
Hide resolved
|
||
'meta' => (array) $stream_meta, | ||
); | ||
|
||
|
@@ -174,12 +162,6 @@ public function is_record_excluded( $connector, $context, $action, $user = null, | |
$user = wp_get_current_user(); | ||
} | ||
|
||
if ( is_null( $ip ) ) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We expect all of the data arriving here to be sanitized and checked already. |
||
$ip = $this->ip_address; | ||
} else { | ||
$ip = wp_stream_filter_var( $ip, FILTER_VALIDATE_IP ); | ||
} | ||
|
||
if ( ! empty( $user->roles ) ) { | ||
$roles = array_values( $user->roles ); | ||
$role = $roles[0]; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -90,6 +90,13 @@ class Plugin { | |
*/ | ||
public $locations = array(); | ||
|
||
/** | ||
* IP address for the current request to be associated with the log entry. | ||
* | ||
* @var null|false|string Valid IP address, null if not set, false if invalid. | ||
*/ | ||
protected $client_ip_address; | ||
|
||
/** | ||
* Class constructor | ||
*/ | ||
|
@@ -138,6 +145,9 @@ public function __construct() { | |
// Load logger class. | ||
$this->log = apply_filters( 'wp_stream_log_handler', new Log( $this ) ); | ||
|
||
// Set the IP address for the current request. | ||
$this->client_ip_address = wp_stream_filter_input( INPUT_SERVER, 'REMOTE_ADDR', FILTER_VALIDATE_IP ); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Make the main plugin class responsible for knowing properties of the current request. Expose the knowledge via a helper method. |
||
|
||
// Load settings and connectors after widgets_init and before the default init priority. | ||
add_action( 'init', array( $this, 'init' ), 9 ); | ||
|
||
|
@@ -315,4 +325,13 @@ public function is_mustuse() { | |
|
||
return false; | ||
} | ||
|
||
/** | ||
* Get the IP address for the current request. | ||
* | ||
* @return false|null|string Valid IP address, null if not set, false if invalid. | ||
*/ | ||
public function get_client_ip_address() { | ||
return apply_filters( 'wp_stream_client_ip_address', $this->client_ip_address ); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Account for calls to this method when
get_current_screen()
is not available.