Skip to content
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

Plugin to exclude the webmaster based on IP or IP range #5463

Closed
mattab opened this issue Jan 3, 2008 · 9 comments
Closed

Plugin to exclude the webmaster based on IP or IP range #5463

mattab opened this issue Jan 3, 2008 · 9 comments
Assignees
Labels
Critical Indicates the severity of an issue is very critical and the issue has a very high priority. Enhancement For new feature suggestions that enhance Matomo's capabilities or add a new report, new API etc.
Milestone

Comments

@mattab
Copy link
Member

mattab commented Jan 3, 2008

We want to be able to exclude a list of IPs, and/or a list of IP ranges (using ..123.34 notation) from being tracked by Piwik.

The list of IPs to exclude would be stored in the website cache file (where goals and alias URLs are stored).

In the UI, the feature should be available
- general exclusion list (only available to the super user): would apply to all website registered in Piwik, would need their own cache file (that applies to all websites)
- for each website, we can define a list of IPs. All IPs in this list + in the general list are checked at each Piwik request.

As an inline help, the UI would show the user current IP that he could copy paste in the list.

The UI would call the API in JSON (like the existing screens).

The UI for this feature should be designed to be part of a “preference page” for a website, as we need to add several new preferences for each website: #41, #42, #43, #56. Ideally, all the UI would be ajax (very quick to go from the list of websites in the admin UI, to load one website details page, to come back to the list of websites).

Outstanding question: should it be in the SiteManager plugin, or a new plugin? Should it be part of the core (to minimize overhead of loading plugins at Tracker time…)

@robocoder
Copy link
Contributor

There's a special case where the IP (or IP range) to be excluded falls within these reserved ranges:

* 10.0.0.0 - 10.255.255.255
* 172.16.0.0 - 172.31.255.255
* 192.168.0.0 - 192.168.255.255 

I'm going to roll the requirements in from #567. Visitors with HTTP_CLIENT_IP or HTTP_X_FORWARDED_FOR, IP addresses in the private IP address ranges should instead use the first public address in the list, falling back to $_SERVER!['REMOTE_ADDR'].

Also, it looks like there are a couple of unreachable codepaths in the current implementation of getIp() which should be reviewed.

@robocoder
Copy link
Contributor

In http://forum.piwik.org/index.php?showtopic=1540 there's a mod to Provider to map local IP addresses to internal "provider" names, eg "net1", "net2", ...

@mattab
Copy link
Member Author

mattab commented Dec 21, 2009

this has become by far the most requested feature - receiving messages twice a day asking for this feature. We should push it in the product.

@mattab
Copy link
Member Author

mattab commented Mar 22, 2010

(In [1970]) Fixes #5463

  • modify websites admin UI, API to add a column Exclude IPs
  • IPs contain wildcards, unlimited IPs per website
  • below the website table, added a "Global IP exclude" list. Ips there are excluded from all websites automatically.
  • IPs are stored in the tracker cache file for fast access at Tracking time.
  • added new field in website table "excluded_ips"
  • refactored the ajax loading/error display to allow multiple loading/error div per page

@mattab
Copy link
Member Author

mattab commented Mar 22, 2010

Known limitations

  • does not work with IPv6 addresses
  • I haven't researched the getIpString() function as explained by Anthon in [#comment:10] - feel free to pick it up from there

@robocoder
Copy link
Contributor

I see the code uses ip2long() and <= => for comparisons. This smells like a latent bug. Since longs are signed, we have to make sure IPs in the range of 128.0.0.0-255.255.255.255 are handled correctly. Re-opening so we can add isVisitorIpExcluded() testable via unit tests.

re: #comment:10, I've re-opened #567.

@mattab
Copy link
Member Author

mattab commented Mar 22, 2010

ouch you're right, thanks for the review!

@mattab
Copy link
Member Author

mattab commented Mar 22, 2010

(In [1972]) Fixes #5463
Adding tests that prove that the code was working as expected (I got lucky :)
the IP 255.255.255.* is stored in the config file as:
array (
0 => -256,
1 => -1,
),
which passes the test >= && <= as expected

@robocoder
Copy link
Contributor

(In [1977]) refs #5463 - getIp() returns a stringified, unsigned number; changed unit test to match getIp()'s behaviour; SitesManager converts min/max to non-negative numbers

@mattab mattab added this to the Piwik 0.6 milestone Jul 8, 2014
@mattab mattab self-assigned this Jul 8, 2014
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Critical Indicates the severity of an issue is very critical and the issue has a very high priority. Enhancement For new feature suggestions that enhance Matomo's capabilities or add a new report, new API etc.
Projects
None yet
Development

No branches or pull requests

2 participants