Problem/Motivation

In some cases, there are situations where you want to exclude looking up some IP address to reduce load or prevent excessive requests. Such IPs can be A monitoring service that pings the site. An internal IP of a company

Proposed resolution

It would be nice to add some a configuration text area in Smart IP config page, to list IPs to exclude.
Those IPs will then be skipped from being queried.

User interface changes

Add configuration text area in /admin/config/people/smart_ip configuration page.

API changes

TBD, but probably minimal or none.
We only need to add an if statement in geolocateUser() function to skip looking up those IPs.

Comments

Mohammed J. Razem created an issue. See original summary.

mohammed j. razem’s picture

Issue summary: View changes
arpeggio’s picture

I'll work on this feature on my free time. Of course, patches are always welcome. Thank you for the suggestion.

milosr’s picture

Status: Active » Needs review
StatusFileSize
new3.47 KB
mohammed j. razem’s picture

Here's another patch (based on what @MilosR did.
It includes minor naming changes, and some missing methods such as actually saving the form config.

I also moved the check of excluded IPs to query(). However, I do not think this is the right way. But hoping this can give someone a head start to get the feature finalized.

omar alahmed’s picture

StatusFileSize
new2.37 KB

Replace array_search with in_array in #5 as array_search returns the index of the found IP so if it was the first index will be 0 which means FALSE and won't match the condition! But in_array returns TRUE if IP is found in the array, FALSE otherwise.

mohammed j. razem’s picture

Status: Needs review » Needs work
+++ b/src/SmartIp.php
@@ -29,6 +29,14 @@ class SmartIp {
+    if (is_array($excluded_ips) && in_array($data['location']['ipAddress'], $excluded_ips)) {
+      return array();
+    }

In some cases, $data array is not set. For example when IP detection is removed from certain roles.

This will result in a PHP Notice: Undefined variable: data in Drupal\smart_ip\SmartIp::query() (line 36 of smart_ip/src/SmartIp.php)

This needs an isset() check on the $data variable before even querying the $excluded_ips config.

omar alahmed’s picture

StatusFileSize
new2.33 KB

The following patch fixes the notice.

  • arpeggio committed 1e2b475 on 8.x-3.x
    Issue #3107344 by Omar Alahmed, Mohammed J. Razem, MilosR, arpeggio:...

  • arpeggio committed 5312746 on 8.x-4.x
    Issue #3107344 by Omar Alahmed, Mohammed J. Razem, MilosR, arpeggio:...
arpeggio’s picture

Status: Needs work » Fixed

I have improved the checking of excluded IPs logic and changed the comma delimiter to new line (this is to be consistent with the allowed_pages form). Thanks to all.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.