From 17f91b1c5b50b0910659dd67c929fde8e91ac62a Mon Sep 17 00:00:00 2001 From: sgiehl Date: Tue, 4 Aug 2026 12:02:03 +0200 Subject: [PATCH 1/5] Add optional removal of the visitor IP before forwarding to Matomo Some organisations must ensure the visitor IP never reaches the analytics application at all, not even to be anonymised there. Matomo anonymises the IP inside Matomo, so the full IP arrives first. $REMOVE_VISITOR_IP moves that boundary out to the proxy. When enabled the proxy forwards the placeholder cip=0.0.0.0 instead of the visitor IP and ignores $http_ip_forward_header. The placeholder is sent rather than nothing because Matomo only falls back to the connection IP when cip is empty - omitting it would make Matomo record the proxy's own IP and report the proxy's location as real visitor data. The option governs the IP the proxy contributes. A request supplying its own cip is forwarded untouched: Matomo honors a cip only on an authenticated request, so such a request is a deliberate decision to track a specific IP, and clientProvidesAuthParams() already withholds our token from it. This needs no change to the bulk path at all - clean entries already receive the forwarded IP, cip-bearing entries are already left alone. getVisitIp() now has a single caller, getVisitIpToForward(), so the real IP cannot leave the proxy by any path when the option is on. Also aligns the bulk-request detection with Matomo's Requests::isUsingBulkRequest(), which uses a truthy strpos check: a "requests" marker at offset 0 is not a bulk request there, so treating it as one here meant skipping cip injection on a request Matomo tracks as an ordinary one, silently losing the visitor IP. The option is off by default; existing deployments are unaffected. --- .github/config.php | 5 + README.md | 45 ++++- config.php.example | 19 ++ proxy.php | 36 +++- tests/ProxyTest.php | 391 ++++++++++++++++++++++++++++++++++++++++ tests/server/matomo.php | 2 +- 6 files changed, 489 insertions(+), 9 deletions(-) diff --git a/.github/config.php b/.github/config.php index 44af390..fa4a713 100644 --- a/.github/config.php +++ b/.github/config.php @@ -17,6 +17,11 @@ $http_ip_forward_header = $_SERVER['HTTP_X_TEST_IP_FORWARD_HEADER']; } +// Exercise removal of the visitor IP (any truthy value enables it). +if ($isTestServer && isset($_SERVER['HTTP_X_TEST_REMOVE_VISITOR_IP'])) { + $REMOVE_VISITOR_IP = $_SERVER['HTTP_X_TEST_REMOVE_VISITOR_IP']; +} + // Exercise cookie-allowlist filtering (comma-separated entries; empty value = explicit empty allowlist). if ($isTestServer && isset($_SERVER['HTTP_X_TEST_COOKIE_ALLOWLIST'])) { $COOKIE_ALLOWLIST = $_SERVER['HTTP_X_TEST_COOKIE_ALLOWLIST'] === '' diff --git a/README.md b/README.md index 11969f1..a46e5df 100644 --- a/README.md +++ b/README.md @@ -116,13 +116,51 @@ You may force the proxy script to use a particular User-Agent by editing the `$ ### Visitor IP forwarding -Because the proxy sits between your visitors and Matomo, it has to tell Matomo the real visitor IP — otherwise Matomo would record the proxy's IP. There are two ways this works: +Because the proxy sits between your visitors and Matomo, it has to tell Matomo the real visitor IP — otherwise Matomo would record the proxy's IP. There are three ways this works: - **Default — via `cip` + `token_auth`:** the proxy sends the visitor IP to Matomo as the `cip` tracking parameter, authorized by the `$TOKEN_AUTH` you configured (this is why the proxy user needs **write** or **admin** permission). Works out of the box with no Matomo-side configuration, for both single requests and bulk requests (the Matomo JavaScript tracker batches several actions into a single bulk request by default). - **Header-only — via `$http_ip_forward_header`:** set `$http_ip_forward_header` in `config.php` (for example to `X-Forwarded-For`) to forward the visitor IP in that header instead. In this mode the proxy injects **no** `cip`/`token_auth` at all and relies solely on the header for the visitor IP — so it doesn't even need a write/admin token. **This only works if Matomo is configured to trust the header:** both the web server in front of Matomo (Apache [mod_remoteip](https://httpd.apache.org/docs/2.4/mod/mod_remoteip.html), nginx [realip](https://www.nginx.com/resources/wiki/start/topics/examples/forwarded/)) **and** Matomo's trusted-proxy settings (`proxy_client_headers[]` / `proxy_ips[]` in its `config.ini.php`). If it isn't, Matomo records the proxy's IP for every visitor. +- **Not at all — via `$REMOVE_VISITOR_IP`:** the visitor IP is never sent to Matomo. See [Removing the visitor IP](#removing-the-visitor-ip) below. + > ⚠️ **Breaking change:** previously `$http_ip_forward_header` was sent *in addition* to `cip`+`token_auth`; the proxy now treats it as the *sole* IP mechanism and injects nothing else. If you already set it, make sure Matomo's trusted-proxy configuration above is in place — otherwise leave it empty to keep using `cip`. +### Removing the visitor IP + +Some organisations are required to ensure the visitor IP never reaches the analytics application at all — not even to be anonymised there. Matomo's own IP anonymisation runs inside Matomo, so the full IP arrives there first. Setting `$REMOVE_VISITOR_IP = true;` in `config.php` moves that boundary out to the proxy. + +That is the only setup step — nothing needs to change in Matomo. With the option enabled: + +- the proxy sends `cip=0.0.0.0` instead of the visitor IP, for single and bulk tracking requests alike +- the proxy never reads the visitor IP at all, so it cannot leak it into a header either +- `$http_ip_forward_header` is ignored, since it would send the IP straight back, and a warning is written to the PHP error log for as long as both are configured — clear `$http_ip_forward_header` to stop it + +> ⚠️ **The write/admin `$TOKEN_AUTH` is still required.** Matomo only honors `cip` on an authenticated request; otherwise it **rejects the request with HTTP 400 and records nothing at all**. So removing the token doesn't degrade your data, it discards it. + +> Note: the placeholder is `0.0.0.0` rather than no `cip` at all because Matomo falls back to the IP of the connection whenever `cip` is empty. Sending nothing would make Matomo record the proxy's IP and report the proxy's location as though it were real visitor data. + +**What this option does not do.** It governs the IP *the proxy contributes*. A request that supplies its own `cip` is forwarded untouched, because Matomo only honors a `cip` on an authenticated request — so such a request is a deliberate decision to track a specific IP, made by something holding a valid token. The browser JavaScript tracker never sends `cip`, so ordinary visitor traffic is unaffected by this distinction. Two consequences worth knowing: + +- If you also want to forbid server-side integrations from submitting IPs, that belongs in those integrations, or in which tokens you issue — the proxy will not overrule them. +- This assumes Matomo's `tracking_requests_require_authentication` is at its default of `1`. If it has been set to `0`, Matomo honors an unauthenticated `cip`, and then anything could submit an IP. + +Explicit location parameters (`lat`, `long`, `city`, `region`, `country`) are likewise not removed — Matomo already requires authentication for those. For cookies that might embed an IP, see [Cookie forwarding](#cookie-forwarding) below; a `Cookie` header is forwarded as-is unless you set `$COOKIE_ALLOWLIST`. + +#### Impact on your Matomo reports + +| Area | Effect | +|------|--------| +| Visits, pageviews, events, goals, ecommerce, campaigns, referrers, search engines, channels, downloads, outlinks, site search, content | Not directly affected | +| Location reports and maps, location-based segments, dashboards and scheduled reports | "Unknown" — though Matomo may still guess a country from the visitor's `Accept-Language` header | +| Visitor IP column and IP-based segments | `0.0.0.0` for every visit | +| IP exclusions, IP-based spam/bot blocking | Can no longer identify individual visitors | +| Provider / ISP reports | "Unknown" (the reverse DNS lookup is skipped) | +| Visits, Unique Visitors, Returning Visitors, bounce rate, visit duration | Less accurate **when cookies are unavailable**: Matomo uses the IP as part of its cookieless visitor fingerprint, so visitors sharing an OS, browser and language may be merged into one visit. With tracking cookies enabled the visitor ID takes precedence and the effect is limited. | +| Goals, funnels, ecommerce attribution | Affected only where the above merges separate visitors | +| QueuedTracking | Requests without a visitor ID are sharded by IP, so they all land in one queue instead of being spread across the configured number | + +> ⚠️ **TrackingSpamPrevention: check the *maximum actions per visit* setting before enabling this.** Because every visit now reports `0.0.0.0`, the first visitor to exceed that limit causes `0.0.0.0/32` to be added to the plugin's blocked IP ranges — after which **every** visit is excluded and tracking stops entirely, silently. The setting is unlimited by default, so this only affects you if it has been changed. If you use it, either unset it or add `0.0.0.0` to the plugin's always-allowed IP ranges. + ### Cookie forwarding By default, the proxy forwards the visitor's entire `Cookie` header to Matomo unchanged. If your site also sets other cookies (session, consent-management, A/B testing, etc.) alongside Matomo's, those are forwarded too. @@ -176,6 +214,11 @@ if ($isTestServer && !empty($_SERVER['HTTP_X_TEST_IP_FORWARD_HEADER'])) { $http_ip_forward_header = $_SERVER['HTTP_X_TEST_IP_FORWARD_HEADER']; } +// Exercise removal of the visitor IP (any truthy value enables it). +if ($isTestServer && isset($_SERVER['HTTP_X_TEST_REMOVE_VISITOR_IP'])) { + $REMOVE_VISITOR_IP = $_SERVER['HTTP_X_TEST_REMOVE_VISITOR_IP']; +} + // Exercise cookie-allowlist filtering (comma-separated entries; empty value = explicit empty allowlist). if ($isTestServer && isset($_SERVER['HTTP_X_TEST_COOKIE_ALLOWLIST'])) { $COOKIE_ALLOWLIST = $_SERVER['HTTP_X_TEST_COOKIE_ALLOWLIST'] === '' diff --git a/config.php.example b/config.php.example index 2e21430..56a6031 100644 --- a/config.php.example +++ b/config.php.example @@ -50,6 +50,25 @@ $user_agent = ''; // $http_ip_forward_header = ''; +// Set this to true to stop sending the visitor IP to Matomo altogether. The proxy then forwards the +// placeholder 0.0.0.0 as `cip` instead of the real IP, and $http_ip_forward_header above is ignored. +// Intended for deployments where the visitor IP must not reach Matomo at all, even before +// anonymisation. Any non-empty value other than '0' enables it. +// +// Your $TOKEN_AUTH is still required: Matomo rejects an unauthenticated `cip` with an HTTP 400 and +// records nothing at all, so removing the token discards your traffic rather than degrading it. +// +// This governs the IP the proxy itself contributes. A request that supplies its own `cip` is left +// untouched, because Matomo only honors one for an authenticated request - so it is a deliberate +// choice to track a specific IP. Location parameters a client supplies explicitly (lat, long, city, +// region, country) are likewise not removed. +// +// Impacts: Location and Provider reports become "Unknown", the visitor IP shows as 0.0.0.0, and +// IP-based exclusions and bot blocking can no longer identify individual visitors. Visit counts also +// become less accurate for visitors without cookies. See README.md for the full list and for an +// important warning about the TrackingSpamPrevention plugin. +$REMOVE_VISITOR_IP = false; + // By default, the proxy forwards the visitor's entire Cookie header to Matomo unchanged, which // also forwards unrelated site cookies (session, consent tools, A/B testing, etc.). // diff --git a/proxy.php b/proxy.php index ceba137..8eb5492 100644 --- a/proxy.php +++ b/proxy.php @@ -54,6 +54,15 @@ $user_agent = arrayValue($_SERVER, 'HTTP_USER_AGENT', ''); } +// If enabled, the visitor IP is never sent to Matomo (see README.md). +$REMOVE_VISITOR_IP = !empty($REMOVE_VISITOR_IP); + +// Removing the visitor IP takes precedence: the header would send it straight back. +if ($REMOVE_VISITOR_IP && !empty($http_ip_forward_header)) { + error_log('$REMOVE_VISITOR_IP is enabled, so $http_ip_forward_header is ignored.'); + $http_ip_forward_header = ''; +} + // ----------------------------- // DO NOT MODIFY BELOW THIS LINE // ----------------------------- @@ -120,9 +129,10 @@ // Without an IP-forward header, send the visitor IP as `cip` authorized by our token_auth - but // only when the client sent no token_auth or auth-protected param, so we never authorize its override. if (empty($http_ip_forward_header)) { - // Same bulk detection as Matomo's Requests::isUsingBulkRequest (both quote variants). - $isBulk = $rawPostBody !== '' - && (strpos($rawPostBody, '"requests"') !== false || strpos($rawPostBody, "'requests'") !== false); + // Same bulk detection as Matomo's Requests::isUsingBulkRequest, down to its truthy strpos + // check: a marker at offset 0 is not bulk there, so it must not be bulk here either. + $isBulk = !empty($rawPostBody) + && (strpos($rawPostBody, '"requests"') || strpos($rawPostBody, "'requests'")); if ($isBulk) { // Matomo reads the bulk token only from the JSON body, so pass any URL token_auth down to @@ -130,12 +140,12 @@ $clientUrlToken = (isset($_GET['token_auth']) && is_string($_GET['token_auth']) && $_GET['token_auth'] !== '') ? $_GET['token_auth'] : null; - $forwardPostBody = injectVisitIpIntoBulkRequest($rawPostBody, getVisitIp(), $TOKEN_AUTH, $clientUrlToken); + $forwardPostBody = injectVisitIpIntoBulkRequest($rawPostBody, getVisitIpToForward(), $TOKEN_AUTH, $clientUrlToken); // The batch token now lives in the JSON body; never also send one in the forwarded query. unset($_GET['token_auth']); } else { if (!isset($_GET['cip']) && !isset($_POST['cip'])) { - $extraQueryParams['cip'] = getVisitIp(); + $extraQueryParams['cip'] = getVisitIpToForward(); } if (!clientProvidesAuthParams($_GET) && !clientProvidesAuthParams($_POST)) { // Drop any empty/array token_auth the client sent so it can't clobber ours when @@ -255,6 +265,18 @@ function getVisitIp() return arrayValue($_SERVER, 'REMOTE_ADDR'); } +function getVisitIpToForward() +{ + global $REMOVE_VISITOR_IP; + + // Matomo falls back to the connection IP - ours - when cip is empty, so send a placeholder. + if ($REMOVE_VISITOR_IP) { + return '0.0.0.0'; + } + + return getVisitIp(); +} + function transformHeaderLine($headerLine) { // if we're not on an https protocol, make sure cookies do not have 'secure;' @@ -383,7 +405,7 @@ function getHttpContentAndStatus($url, $timeout, $user_agent, $postBody = '') // Forward the visitor IP via the configured header, for every request method. if (!empty($http_ip_forward_header)) { - $visitIp = getVisitIp(); + $visitIp = getVisitIpToForward(); $stream_options['http']['header'][] = "$http_ip_forward_header: $visitIp"; } @@ -500,7 +522,7 @@ function withProxyTracking( $tokenAuth, $includeProxyToken ) { - // The entry is clean (no cip of its own), so set the real visitor IP. + // The entry is clean (no cip of its own), so set the IP we forward. $params['cip'] = $visitIp; // Lend our token only when the caller decided to; otherwise a client token authorizes the cip. diff --git a/tests/ProxyTest.php b/tests/ProxyTest.php index b66e91a..5b1d703 100644 --- a/tests/ProxyTest.php +++ b/tests/ProxyTest.php @@ -1176,6 +1176,397 @@ public function test_non_array_cookie_allowlist_fails_closed_and_drops_all_cooki $this->assertEquals($expected, $responseBody); } + public function test_visitor_ip_removal_sends_placeholder_cip_with_proxy_token() + { + $response = $this->send('foo=bar', null, null, ['X-Test-Remove-Visitor-Ip' => '1']); + + $responseBody = $this->getBody($response); + + $expected = << '0.0.0.0', + 'token_auth' => '', + 'foo' => 'bar', +) +RESPONSE; + + $this->assertEquals(200, $response->getStatusCode()); + $this->assertEquals($expected, $responseBody); + } + + public function test_visitor_ip_removal_ignores_client_ip_headers() + { + $headers = [ + 'X-Forwarded-For' => '8.8.8.8', + 'Client-Ip' => '6.6.6.6', + 'Cf-Connecting-Ip' => '6.6.6.6', + 'X-Test-Remove-Visitor-Ip' => '1', + ]; + $response = $this->send('foo=bar', null, null, $headers); + + $responseBody = $this->getBody($response); + + // The headers getVisitIp() would have read are neither used nor forwarded, so no header + // block is echoed at all. + $expected = << '0.0.0.0', + 'token_auth' => '', + 'foo' => 'bar', +) +RESPONSE; + + $this->assertEquals(200, $response->getStatusCode()); + $this->assertEquals($expected, $responseBody); + $this->assertStringNotContainsString('8.8.8.8', $responseBody); + $this->assertStringNotContainsString('6.6.6.6', $responseBody); + } + + public function test_visitor_ip_removal_leaves_a_client_supplied_cip_untouched() + { + $response = $this->send('idsite=1&cip=6.6.6.6&foo=bar', null, null, ['X-Test-Remove-Visitor-Ip' => '1']); + + $responseBody = $this->getBody($response); + + // A client cip needs a valid token_auth of its own to be honored, so it is a deliberate + // request to track a specific IP: the proxy adds neither a placeholder nor its token. + $expected = << '1', + 'cip' => '6.6.6.6', + 'foo' => 'bar', +) +RESPONSE; + + $this->assertEquals(200, $response->getStatusCode()); + $this->assertEquals($expected, $responseBody); + } + + public function test_visitor_ip_removal_leaves_a_client_supplied_cip_in_the_post_body_untouched() + { + $response = $this->send( + 'foo=bar&raw_input=1', + null, + null, + ['content-type' => 'application/x-www-form-urlencoded', 'X-Test-Remove-Visitor-Ip' => '1'], + null, + 'POST', + 'cip=6.6.6.6&action_name=x' + ); + + $responseBody = $this->getBody($response); + + $expected = << 'bar', + 'raw_input' => '1', +) +array ( + 'cip' => '6.6.6.6', + 'action_name' => 'x', +) +RAW: cip=6.6.6.6&action_name=x +RESPONSE; + + $this->assertEquals(200, $response->getStatusCode()); + $this->assertEquals($expected, $responseBody); + } + + public function test_visitor_ip_removal_with_empty_client_cip_injects_no_placeholder() + { + $response = $this->send('idsite=1&cip=', null, null, ['X-Test-Remove-Visitor-Ip' => '1']); + + $responseBody = $this->getBody($response); + + // An empty cip is still a client-supplied cip, so we leave it alone. Matomo treats it as + // absent and falls back to the connection IP - the proxy's, never the visitor's. + $expected = << '1', + 'cip' => '', +) +RESPONSE; + + $this->assertEquals(200, $response->getStatusCode()); + $this->assertEquals($expected, $responseBody); + } + + public function test_visitor_ip_removal_keeps_explicit_location_params() + { + $response = $this->send('idsite=1&lat=1&long=2', null, null, ['X-Test-Remove-Visitor-Ip' => '1']); + + $responseBody = $this->getBody($response); + + // The option removes the IP, not explicitly supplied location parameters. + $expected = << '0.0.0.0', + 'idsite' => '1', + 'lat' => '1', + 'long' => '2', +) +RESPONSE; + + $this->assertEquals(200, $response->getStatusCode()); + $this->assertEquals($expected, $responseBody); + } + + public function test_visitor_ip_removal_bulk_injects_placeholder_and_top_level_token() + { + $body = '{"requests":["?idsite=1&rec=1&action_name=one"],"send_image":0}'; + + $response = $this->sendBulkWithoutVisitorIp($body); + + $responseBody = $this->getBody($response); + + $this->assertEquals(200, $response->getStatusCode()); + $this->assertStringContainsString('action_name=one&cip=0.0.0.0', $responseBody); + $this->assertStringContainsString('"token_auth":""', $responseBody); + $this->assertStringNotContainsString('127.0.0.1', $responseBody); + } + + public function test_visitor_ip_removal_bulk_leaves_an_entry_with_its_own_cip_untouched() + { + $body = '{"requests":["?idsite=1&rec=1&cip=6.6.6.6"]}'; + + $response = $this->sendBulkWithoutVisitorIp($body); + + $responseBody = $this->getBody($response); + + $this->assertEquals(200, $response->getStatusCode()); + // Entry forwarded verbatim and given no token, exactly as without the option. + $this->assertStringContainsString('"?idsite=1&rec=1&cip=6.6.6.6"', $responseBody); + $this->assertStringNotContainsString('', $responseBody); + $this->assertStringNotContainsString('0.0.0.0', $responseBody); + } + + public function test_visitor_ip_removal_bulk_injects_placeholder_only_into_clean_entries() + { + $body = '{"requests":["?idsite=1&rec=1&action_name=clean","?idsite=1&rec=1&cip=6.6.6.6"]}'; + + $response = $this->sendBulkWithoutVisitorIp($body); + + $responseBody = $this->getBody($response); + + $this->assertEquals(200, $response->getStatusCode()); + $this->assertStringContainsString('action_name=clean&cip=0.0.0.0', $responseBody); + $this->assertStringContainsString('"?idsite=1&rec=1&cip=6.6.6.6"', $responseBody); + $this->assertStringNotContainsString('127.0.0.1', $responseBody); + } + + public function test_visitor_ip_removal_bulk_injects_placeholder_into_clean_object_entry() + { + $body = '{"requests":[{"idsite":"1","rec":"1","action_name":"clean"}]}'; + + $response = $this->sendBulkWithoutVisitorIp($body); + + $responseBody = $this->getBody($response); + + $this->assertEquals(200, $response->getStatusCode()); + $this->assertStringContainsString('"action_name":"clean","cip":"0.0.0.0"', $responseBody); + $this->assertStringContainsString('"token_auth":""', $responseBody); + $this->assertStringNotContainsString('127.0.0.1', $responseBody); + } + + public function test_bulk_marker_at_offset_zero_is_not_treated_as_bulk() + { + // Matomo's Requests::isUsingBulkRequest() uses a truthy strpos check, so a marker at offset + // 0 is not a bulk request there. The proxy must agree, or it skips cip injection on a + // request Matomo tracks as an ordinary one. + $body = '"requests"=x&idsite=1&rec=1'; + + $response = $this->send( + 'raw_input=1', + null, + null, + ['content-type' => 'application/x-www-form-urlencoded'], + null, + 'POST', + $body + ); + + $responseBody = $this->getBody($response); + + $this->assertEquals(200, $response->getStatusCode()); + $this->assertStringContainsString("'cip' => '127.0.0.1'", $responseBody); + $this->assertStringContainsString("'token_auth' => ''", $responseBody); + } + + public function test_visitor_ip_removal_bulk_marker_at_offset_zero_gets_placeholder() + { + $body = '"requests"=x&idsite=1&rec=1'; + + $response = $this->send( + 'raw_input=1', + null, + null, + ['content-type' => 'application/x-www-form-urlencoded', 'X-Test-Remove-Visitor-Ip' => '1'], + null, + 'POST', + $body + ); + + $responseBody = $this->getBody($response); + + $this->assertEquals(200, $response->getStatusCode()); + $this->assertStringContainsString("'cip' => '0.0.0.0'", $responseBody); + $this->assertStringNotContainsString('127.0.0.1', $responseBody); + } + + public function test_visitor_ip_removal_takes_precedence_over_forward_header() + { + $headers = [ + 'X-Test-Ip-Forward-Header' => 'X-Forwarded-For', + 'X-Test-Remove-Visitor-Ip' => '1', + ]; + $response = $this->send('idsite=1&action_name=clean', null, null, $headers); + + $responseBody = $this->getBody($response); + + // The forward header is ignored, so the proxy is back on the cip path - with the placeholder. + $expected = << '0.0.0.0', + 'token_auth' => '', + 'idsite' => '1', + 'action_name' => 'clean', +) +RESPONSE; + + $this->assertEquals(200, $response->getStatusCode()); + $this->assertEquals($expected, $responseBody); + } + + public function test_visitor_ip_removal_takes_precedence_over_forward_header_for_bulk() + { + $headers = [ + 'content-type' => 'application/x-www-form-urlencoded', + 'X-Test-Ip-Forward-Header' => 'X-Forwarded-For', + 'X-Test-Remove-Visitor-Ip' => '1', + ]; + $response = $this->send( + 'raw_input=1', + null, + null, + $headers, + null, + 'POST', + '{"requests":["?idsite=1&rec=1&action_name=one"]}' + ); + + $responseBody = $this->getBody($response); + + $this->assertEquals(200, $response->getStatusCode()); + $this->assertStringContainsString('action_name=one&cip=0.0.0.0', $responseBody); + $this->assertStringNotContainsString('X_FORWARDED_FOR', $responseBody); + } + + public function test_forward_header_mode_sends_ip_header_on_plugin_config_endpoint() + { + // Baseline for the next test: without removal, the header is forwarded on this endpoint too. + $headers = ['X-Test-Ip-Forward-Header' => 'X-Forwarded-For']; + $response = $this->send('idsite=35&trackerid=123456', null, null, $headers, '/plugins/HeatmapSessionRecording/configs.php'); + + $responseBody = $this->getBody($response); + + $this->assertEquals(200, $response->getStatusCode()); + $this->assertStringContainsString("'X_FORWARDED_FOR' => '127.0.0.1'", $responseBody); + } + + public function test_visitor_ip_removal_sends_no_ip_header_on_plugin_config_endpoint() + { + $headers = [ + 'X-Test-Ip-Forward-Header' => 'X-Forwarded-For', + 'X-Test-Remove-Visitor-Ip' => '1', + ]; + $response = $this->send('idsite=35&trackerid=123456', null, null, $headers, '/plugins/HeatmapSessionRecording/configs.php'); + + $responseBody = $this->getBody($response); + + // No IP header, and no cip either - this endpoint is not a tracking endpoint. + $expected = << '35', + 'trackerid' => '123456', +) +RESPONSE; + + $this->assertEquals(200, $response->getStatusCode()); + $this->assertEquals($expected, $responseBody); + } + + public function test_visitor_ip_removal_sends_no_ip_header_on_opt_out_endpoint() + { + $headers = [ + 'X-Test-Ip-Forward-Header' => 'X-Forwarded-For', + 'X-Test-Remove-Visitor-Ip' => '1', + ]; + $response = $this->send('module=CoreAdminHome&action=optOut', null, null, $headers, '/matomo-proxy.php'); + + $responseBody = $this->getBody($response); + + $this->assertEquals(200, $response->getStatusCode()); + $this->assertStringNotContainsString('X_FORWARDED_FOR', $responseBody); + } + + public function test_visitor_ip_removal_still_serves_matomo_js() + { + $response = $this->send(null, null, null, ['X-Test-Remove-Visitor-Ip' => '1']); + + $this->assertEquals(200, $response->getStatusCode()); + $this->assertEquals('this is matomo.js', $response->getBody()->getContents()); + } + + public function test_falsy_remove_visitor_ip_keeps_forwarding_the_visitor_ip() + { + $response = $this->send('foo=bar', null, null, ['X-Test-Remove-Visitor-Ip' => '0']); + + $responseBody = $this->getBody($response); + + $expected = << '127.0.0.1', + 'token_auth' => '', + 'foo' => 'bar', +) +RESPONSE; + + $this->assertEquals(200, $response->getStatusCode()); + $this->assertEquals($expected, $responseBody); + } + + public function test_truthy_string_remove_visitor_ip_enables_removal() + { + $response = $this->send('foo=bar', null, null, ['X-Test-Remove-Visitor-Ip' => 'yes']); + + $responseBody = $this->getBody($response); + + $expected = << '0.0.0.0', + 'token_auth' => '', + 'foo' => 'bar', +) +RESPONSE; + + $this->assertEquals(200, $response->getStatusCode()); + $this->assertEquals($expected, $responseBody); + } + + private function sendBulkWithoutVisitorIp($body) + { + return $this->send( + 'raw_input=1', + null, + null, + ['content-type' => 'application/x-www-form-urlencoded', 'X-Test-Remove-Visitor-Ip' => '1'], + null, + 'POST', + $body + ); + } + private function send($query = null, DateTime $modifiedSince = null, $matomoUrl = null, $addHeaders = null, $path = null, $method = 'GET', $body = null, $forceIpV6 = false) { diff --git a/tests/server/matomo.php b/tests/server/matomo.php index 853d186..e147bac 100644 --- a/tests/server/matomo.php +++ b/tests/server/matomo.php @@ -23,7 +23,7 @@ } $headers = array(); -foreach (array('DNT', 'X_DO_NOT_TRACK', 'X_FORWARDED_FOR', 'COOKIE') as $headerName) { +foreach (array('DNT', 'X_DO_NOT_TRACK', 'X_FORWARDED_FOR', 'X_REAL_IP', 'CLIENT_IP', 'CF_CONNECTING_IP', 'COOKIE') as $headerName) { if (isset($_SERVER['HTTP_' . $headerName])) { $headers[$headerName] = $_SERVER['HTTP_' . $headerName]; } From 6931a6ef5d2b41441a49d1cb70975f6327aec235 Mon Sep 17 00:00:00 2001 From: sgiehl Date: Tue, 4 Aug 2026 14:20:41 +0200 Subject: [PATCH 2/5] Treat an empty or array cip as no cip, and quieten the conflict warning Matomo reads `cip` string-only and falls back to the connection IP for an empty or array value, without raising an error. The proxy however counted any `cip` key as client-supplied, so appending `&cip=` suppressed the injected IP entirely and Matomo recorded the proxy's own address - and geolocated its datacentre - as the visitor's. With $REMOVE_VISITOR_IP that defeats the placeholder the option exists to guarantee; without it, the real visitor IP is silently lost. clientSuppliesVisitIp() now applies the same non-empty-string rule the file already applies to token_auth two lines above, in both the injection guard and clientProvidesAuthParams(). A value Matomo would ignore is dropped from $_GET/$_POST so it cannot win the merge; a real client cip is still forwarded untouched and still receives no token from us. In a bulk batch an empty cip additionally made the entry look auth-protected, demoting the whole batch from a top-level token to per-entry tokens, which a server with bulk_requests_require_authentication rejects outright. The $http_ip_forward_header conflict warning is now only logged when $DEBUG_PROXY is on: it reports a permanent misconfiguration, so a busy proxy was writing one synchronous log line per request indefinitely. Docs: an unauthenticated cip is only an HTTP 400 for a single request; a bulk request - what the JS tracker sends by default - returns HTTP 200 with "tracked":0, so there is no error status to alert on. Also names the TrackingSpamPrevention iprange_allowlist[] ini key rather than implying a UI setting, notes its excluded/included_countries hazard, and corrects the ban threshold from "exceed" to "reach". --- README.md | 9 ++--- config.php.example | 14 ++++---- proxy.php | 30 +++++++++++++--- tests/ProxyTest.php | 83 ++++++++++++++++++++++++++++++++++++++++++--- 4 files changed, 118 insertions(+), 18 deletions(-) diff --git a/README.md b/README.md index a46e5df..7651b8b 100644 --- a/README.md +++ b/README.md @@ -120,7 +120,6 @@ Because the proxy sits between your visitors and Matomo, it has to tell Matomo t - **Default — via `cip` + `token_auth`:** the proxy sends the visitor IP to Matomo as the `cip` tracking parameter, authorized by the `$TOKEN_AUTH` you configured (this is why the proxy user needs **write** or **admin** permission). Works out of the box with no Matomo-side configuration, for both single requests and bulk requests (the Matomo JavaScript tracker batches several actions into a single bulk request by default). - **Header-only — via `$http_ip_forward_header`:** set `$http_ip_forward_header` in `config.php` (for example to `X-Forwarded-For`) to forward the visitor IP in that header instead. In this mode the proxy injects **no** `cip`/`token_auth` at all and relies solely on the header for the visitor IP — so it doesn't even need a write/admin token. **This only works if Matomo is configured to trust the header:** both the web server in front of Matomo (Apache [mod_remoteip](https://httpd.apache.org/docs/2.4/mod/mod_remoteip.html), nginx [realip](https://www.nginx.com/resources/wiki/start/topics/examples/forwarded/)) **and** Matomo's trusted-proxy settings (`proxy_client_headers[]` / `proxy_ips[]` in its `config.ini.php`). If it isn't, Matomo records the proxy's IP for every visitor. - - **Not at all — via `$REMOVE_VISITOR_IP`:** the visitor IP is never sent to Matomo. See [Removing the visitor IP](#removing-the-visitor-ip) below. > ⚠️ **Breaking change:** previously `$http_ip_forward_header` was sent *in addition* to `cip`+`token_auth`; the proxy now treats it as the *sole* IP mechanism and injects nothing else. If you already set it, make sure Matomo's trusted-proxy configuration above is in place — otherwise leave it empty to keep using `cip`. @@ -135,11 +134,11 @@ That is the only setup step — nothing needs to change in Matomo. With the opti - the proxy never reads the visitor IP at all, so it cannot leak it into a header either - `$http_ip_forward_header` is ignored, since it would send the IP straight back, and a warning is written to the PHP error log for as long as both are configured — clear `$http_ip_forward_header` to stop it -> ⚠️ **The write/admin `$TOKEN_AUTH` is still required.** Matomo only honors `cip` on an authenticated request; otherwise it **rejects the request with HTTP 400 and records nothing at all**. So removing the token doesn't degrade your data, it discards it. +> ⚠️ **The write/admin `$TOKEN_AUTH` is still required, and this includes deployments that previously used header-only mode without one.** Matomo only honors `cip` on an authenticated request, and otherwise **records nothing at all** — so removing or downgrading the token discards your data rather than degrading it. Watch out for how that surfaces: a single request is rejected with HTTP 400, but a **bulk** request (what the JavaScript tracker sends by default) comes back as HTTP 200 with `{"status":"success","tracked":0,"invalid":N}`, so there is no error status to alert on. > Note: the placeholder is `0.0.0.0` rather than no `cip` at all because Matomo falls back to the IP of the connection whenever `cip` is empty. Sending nothing would make Matomo record the proxy's IP and report the proxy's location as though it were real visitor data. -**What this option does not do.** It governs the IP *the proxy contributes*. A request that supplies its own `cip` is forwarded untouched, because Matomo only honors a `cip` on an authenticated request — so such a request is a deliberate decision to track a specific IP, made by something holding a valid token. The browser JavaScript tracker never sends `cip`, so ordinary visitor traffic is unaffected by this distinction. Two consequences worth knowing: +**What this option does not do.** It governs the IP *the proxy contributes*. A request that supplies its own `cip` is forwarded untouched, because Matomo only honors a `cip` on an authenticated request — so such a request is a deliberate decision to track a specific IP, made by something holding a valid token. "Supplies its own `cip`" means a non-empty one: an empty or array-valued `cip` is something Matomo ignores in favour of the connection IP, so the proxy replaces it with the placeholder rather than letting its own IP be recorded. The browser JavaScript tracker never sends `cip` at all, so ordinary visitor traffic is unaffected by this distinction. Two consequences worth knowing: - If you also want to forbid server-side integrations from submitting IPs, that belongs in those integrations, or in which tokens you issue — the proxy will not overrule them. - This assumes Matomo's `tracking_requests_require_authentication` is at its default of `1`. If it has been set to `0`, Matomo honors an unauthenticated `cip`, and then anything could submit an IP. @@ -159,7 +158,9 @@ Explicit location parameters (`lat`, `long`, `city`, `region`, `country`) are li | Goals, funnels, ecommerce attribution | Affected only where the above merges separate visitors | | QueuedTracking | Requests without a visitor ID are sharded by IP, so they all land in one queue instead of being spread across the configured number | -> ⚠️ **TrackingSpamPrevention: check the *maximum actions per visit* setting before enabling this.** Because every visit now reports `0.0.0.0`, the first visitor to exceed that limit causes `0.0.0.0/32` to be added to the plugin's blocked IP ranges — after which **every** visit is excluded and tracking stops entirely, silently. The setting is unlimited by default, so this only affects you if it has been changed. If you use it, either unset it or add `0.0.0.0` to the plugin's always-allowed IP ranges. +> ⚠️ **TrackingSpamPrevention needs checking before you enable this.** Because every visit now reports `0.0.0.0`, the first visitor to *reach* the plugin's *maximum actions per visit* limit causes `0.0.0.0/32` to be added to its blocked IP ranges — after which **every** visit is excluded and tracking stops entirely, silently. That setting is unlimited by default, so this only bites if it has been changed; if you use it, either unset it or add `0.0.0.0` to `iprange_allowlist[]` under `[TrackingSpamPrevention]` in Matomo's `config.ini.php`. +> +> The same plugin's *excluded/included countries* settings are affected too: with no usable IP, the country comes from Matomo's `Accept-Language` guess and is `xx` when it cannot be determined. With *included countries* configured and `xx` not among them, every such visit is excluded — the same silent stoppage. ### Cookie forwarding diff --git a/config.php.example b/config.php.example index 56a6031..efc39da 100644 --- a/config.php.example +++ b/config.php.example @@ -55,13 +55,15 @@ $http_ip_forward_header = ''; // Intended for deployments where the visitor IP must not reach Matomo at all, even before // anonymisation. Any non-empty value other than '0' enables it. // -// Your $TOKEN_AUTH is still required: Matomo rejects an unauthenticated `cip` with an HTTP 400 and -// records nothing at all, so removing the token discards your traffic rather than degrading it. +// Your $TOKEN_AUTH is still required: Matomo records nothing at all for an unauthenticated `cip`, +// so removing the token discards your traffic rather than degrading it. A single request is rejected +// with HTTP 400, but a bulk request - what the JS tracker sends by default - returns HTTP 200 with +// "tracked":0, so there is no error status to alert on. // -// This governs the IP the proxy itself contributes. A request that supplies its own `cip` is left -// untouched, because Matomo only honors one for an authenticated request - so it is a deliberate -// choice to track a specific IP. Location parameters a client supplies explicitly (lat, long, city, -// region, country) are likewise not removed. +// This governs the IP the proxy itself contributes. A request supplying its own non-empty `cip` is +// left untouched, because Matomo only honors one for an authenticated request - so it is a +// deliberate choice to track a specific IP. Location parameters a client supplies explicitly (lat, +// long, city, region, country) are likewise not removed. // // Impacts: Location and Provider reports become "Unknown", the visitor IP shows as 0.0.0.0, and // IP-based exclusions and bot blocking can no longer identify individual visitors. Visit counts also diff --git a/proxy.php b/proxy.php index 8eb5492..081c80c 100644 --- a/proxy.php +++ b/proxy.php @@ -57,9 +57,13 @@ // If enabled, the visitor IP is never sent to Matomo (see README.md). $REMOVE_VISITOR_IP = !empty($REMOVE_VISITOR_IP); -// Removing the visitor IP takes precedence: the header would send it straight back. +// Removing the visitor IP takes precedence: the header would send it straight back. Only reported +// when debugging - this is a permanent misconfiguration, so logging it per request would flood the +// error log of a busy proxy. if ($REMOVE_VISITOR_IP && !empty($http_ip_forward_header)) { - error_log('$REMOVE_VISITOR_IP is enabled, so $http_ip_forward_header is ignored.'); + if ($DEBUG_PROXY) { + error_log('$REMOVE_VISITOR_IP is enabled, so $http_ip_forward_header is ignored.'); + } $http_ip_forward_header = ''; } @@ -144,7 +148,10 @@ // The batch token now lives in the JSON body; never also send one in the forwarded query. unset($_GET['token_auth']); } else { - if (!isset($_GET['cip']) && !isset($_POST['cip'])) { + if (!clientSuppliesVisitIp($_GET) && !clientSuppliesVisitIp($_POST)) { + // Drop an empty/array cip, which Matomo ignores anyway, so it can't clobber ours + // when $_GET is merged below (array_merge lets $_GET win on key collision). + unset($_GET['cip'], $_POST['cip']); $extraQueryParams['cip'] = getVisitIpToForward(); } if (!clientProvidesAuthParams($_GET) && !clientProvidesAuthParams($_POST)) { @@ -490,6 +497,16 @@ function arrayValue($array, $key, $value = null) return $value; } +function clientSuppliesVisitIp($params) +{ + // Only a non-empty string cip is read by Matomo; an empty or array value makes it fall back to + // the connection IP instead, so we must not treat those as a client-supplied IP either. + return is_array($params) + && isset($params['cip']) + && is_string($params['cip']) + && $params['cip'] !== ''; +} + function clientProvidesAuthParams($params) { if (!is_array($params)) { @@ -502,9 +519,14 @@ function clientProvidesAuthParams($params) return true; } + // Same reasoning for cip, which Matomo also reads string-only. + if (clientSuppliesVisitIp($params)) { + return true; + } + // Params Matomo only honors for an authenticated request. Checked by key presence // (type-agnostic) so it cannot be evaded with array/empty values. - $overrideParams = array('cdt', 'cdo', 'country', 'region', 'city', 'lat', 'long', 'cip'); + $overrideParams = array('cdt', 'cdo', 'country', 'region', 'city', 'lat', 'long'); foreach ($overrideParams as $param) { if (array_key_exists($param, $params)) { diff --git a/tests/ProxyTest.php b/tests/ProxyTest.php index 5b1d703..6d9fe02 100644 --- a/tests/ProxyTest.php +++ b/tests/ProxyTest.php @@ -403,6 +403,45 @@ public function test_array_token_auth_with_override_does_not_receive_proxy_token $this->assertStringNotContainsString('', $responseBody); } + public function test_empty_client_cip_is_replaced_by_the_visitor_ip() + { + // Matomo reads cip string-only and falls back to the connection IP for an empty value, so an + // empty cip is "no cip" - treating it as a client override would record the proxy's own IP. + $response = $this->send('idsite=1&cip='); + + $responseBody = $this->getBody($response); + + $expected = << '127.0.0.1', + 'token_auth' => '', + 'idsite' => '1', +) +RESPONSE; + + $this->assertEquals(200, $response->getStatusCode()); + $this->assertEquals($expected, $responseBody); + } + + public function test_array_client_cip_is_replaced_by_the_visitor_ip() + { + $response = $this->send('idsite=1&cip[]=6.6.6.6'); + + $responseBody = $this->getBody($response); + + $expected = << '127.0.0.1', + 'token_auth' => '', + 'idsite' => '1', +) +RESPONSE; + + $this->assertEquals(200, $response->getStatusCode()); + $this->assertEquals($expected, $responseBody); + $this->assertStringNotContainsString('6.6.6.6', $responseBody); + } + public function test_post_requests_forward_body_rebuilt_from_parsed_post() { $response = $this->send( @@ -1272,18 +1311,19 @@ public function test_visitor_ip_removal_leaves_a_client_supplied_cip_in_the_post $this->assertEquals($expected, $responseBody); } - public function test_visitor_ip_removal_with_empty_client_cip_injects_no_placeholder() + public function test_visitor_ip_removal_replaces_empty_client_cip_with_placeholder() { $response = $this->send('idsite=1&cip=', null, null, ['X-Test-Remove-Visitor-Ip' => '1']); $responseBody = $this->getBody($response); - // An empty cip is still a client-supplied cip, so we leave it alone. Matomo treats it as - // absent and falls back to the connection IP - the proxy's, never the visitor's. + // Matomo ignores an empty cip and falls back to the connection IP - the proxy's - so leaving + // it in place would record the proxy's IP and location as the visitor's. $expected = << '0.0.0.0', + 'token_auth' => '', 'idsite' => '1', - 'cip' => '', ) RESPONSE; @@ -1291,6 +1331,41 @@ public function test_visitor_ip_removal_with_empty_client_cip_injects_no_placeho $this->assertEquals($expected, $responseBody); } + public function test_visitor_ip_removal_replaces_array_client_cip_with_placeholder() + { + $response = $this->send('idsite=1&cip[]=6.6.6.6', null, null, ['X-Test-Remove-Visitor-Ip' => '1']); + + $responseBody = $this->getBody($response); + + // Matomo reads cip string-only, so an array value is ignored there too. + $expected = << '0.0.0.0', + 'token_auth' => '', + 'idsite' => '1', +) +RESPONSE; + + $this->assertEquals(200, $response->getStatusCode()); + $this->assertEquals($expected, $responseBody); + $this->assertStringNotContainsString('6.6.6.6', $responseBody); + } + + public function test_visitor_ip_removal_bulk_replaces_empty_client_cip_with_placeholder() + { + $body = '{"requests":["?idsite=1&rec=1&action_name=one&cip="]}'; + + $response = $this->sendBulkWithoutVisitorIp($body); + + $responseBody = $this->getBody($response); + + $this->assertEquals(200, $response->getStatusCode()); + // The entry counts as clean, so it gets the placeholder and the batch keeps its top-level + // token - an empty cip must not demote the batch to per-entry tokens. + $this->assertStringContainsString('cip=0.0.0.0', $responseBody); + $this->assertStringContainsString('"token_auth":""', $responseBody); + } + public function test_visitor_ip_removal_keeps_explicit_location_params() { $response = $this->send('idsite=1&lat=1&long=2', null, null, ['X-Test-Remove-Visitor-Ip' => '1']); From be70c2a306e63f44063119e4d1b3d4e72fdb91a3 Mon Sep 17 00:00:00 2001 From: sgiehl Date: Tue, 4 Aug 2026 14:43:31 +0200 Subject: [PATCH 3/5] Address review findings: docs accuracy, config shape, regression coverage The "Auth-protected tracking parameters" section listed cip among the parameters that always withhold the proxy token, which stopped being true for an empty or array cip in the previous commit. Qualifies it as non-empty, matching clientProvidesAuthParams(). Adds the behaviour-change note the README already uses for this kind of change. Two cases previously ended up with no cip at all, so Matomo recorded the proxy's IP rather than the visitor's, and both now send the visitor IP whether or not $REMOVE_VISITOR_IP is set: an empty or array-valued cip, and a POST body whose bulk marker sits at offset 0. $REMOVE_VISITOR_IP now takes the `if (! isset($X))` default shape used by every neighbouring option, and the conflict resolution - which is logic, not a default - moves below the DO NOT MODIFY marker. Regression coverage for the bulk shapes the proxy must not try to rewrite (undecodable body, entry without a query, list-typed entry) and for POST bodies on the two non-tracking endpoints. The bulk helper now sends an X-Forwarded-For, so those assertions prove getVisitIp()'s header sources are bypassed rather than only that REMOTE_ADDR is unused. Drops the X_REAL_IP echo entry, which no test used and getVisitIp() never reads. --- README.md | 6 +++ proxy.php | 14 ++--- tests/ProxyTest.php | 110 +++++++++++++++++++++++++++++++++++++++- tests/server/matomo.php | 2 +- 4 files changed, 123 insertions(+), 9 deletions(-) diff --git a/README.md b/README.md index 7651b8b..b5cf4d9 100644 --- a/README.md +++ b/README.md @@ -124,6 +124,11 @@ Because the proxy sits between your visitors and Matomo, it has to tell Matomo t > ⚠️ **Breaking change:** previously `$http_ip_forward_header` was sent *in addition* to `cip`+`token_auth`; the proxy now treats it as the *sole* IP mechanism and injects nothing else. If you already set it, make sure Matomo's trusted-proxy configuration above is in place — otherwise leave it empty to keep using `cip`. +> ⚠️ **Behavior change:** two cases that previously ended up with no `cip` at all — so Matomo recorded the proxy's IP instead of the visitor's — now send the visitor IP as normal. This applies whether or not `$REMOVE_VISITOR_IP` is set: +> +> - a request whose `cip` is empty or array-valued, which Matomo ignores in favour of the connection IP, so the proxy now treats it as absent; +> - a `POST` body in which the bulk marker `"requests"` appears at the very start, which Matomo does not treat as a bulk request — the proxy now agrees with it instead of forwarding the body unprocessed. + ### Removing the visitor IP Some organisations are required to ensure the visitor IP never reaches the analytics application at all — not even to be anonymised there. Matomo's own IP anonymisation runs inside Matomo, so the full IP arrives there first. Setting `$REMOVE_VISITOR_IP = true;` in `config.php` moves that boundary out to the proxy. @@ -181,6 +186,7 @@ To restrict this, set `$COOKIE_ALLOWLIST` in `config.php` to an array of cookie Some tracking parameters (`cip`, `cdt`, `cdo`, `country`, `region`, `city`, `lat`, `long`) are only honored by Matomo for an authenticated request. The proxy never lends its `$TOKEN_AUTH` to a request — or to an individual entry of a bulk request — that carries one of these override parameters or its own `token_auth`: - **Carries an override parameter, no token:** forwarded without the proxy's token, so Matomo rejects/skips it exactly as if it had been sent directly without authentication — rather than being silently tracked with the client-supplied override. To set these parameters legitimately, send your own valid `token_auth`. + - For `cip` this applies to a **non-empty** value only. Matomo ignores an empty or array-valued `cip` and falls back to the IP of the connection, so the proxy treats such a value as no `cip` at all: it is dropped and replaced with the IP the proxy would otherwise have sent. - **Carries its own `token_auth`:** the proxy adds no token of its own and lets the client's token govern. It still forwards the visitor IP as `cip`, so that token must have write access to authorize it (otherwise the request/entry is rejected). > ⚠️ **Behavior change:** if you add any of these parameters via `appendToTrackingUrl` (or otherwise) without your own `token_auth`, those requests are now **rejected** by Matomo. Previously the proxy stripped the parameter and tracked the rest of the hit; it no longer does. Send a valid `token_auth` if you need these parameters. diff --git a/proxy.php b/proxy.php index 081c80c..8995b5a 100644 --- a/proxy.php +++ b/proxy.php @@ -54,8 +54,14 @@ $user_agent = arrayValue($_SERVER, 'HTTP_USER_AGENT', ''); } -// If enabled, the visitor IP is never sent to Matomo (see README.md). -$REMOVE_VISITOR_IP = !empty($REMOVE_VISITOR_IP); +// Set to true to never send the visitor IP to Matomo, not even an anonymized one +if (! isset($REMOVE_VISITOR_IP)) { + $REMOVE_VISITOR_IP = false; +} + +// ----------------------------- +// DO NOT MODIFY BELOW THIS LINE +// ----------------------------- // Removing the visitor IP takes precedence: the header would send it straight back. Only reported // when debugging - this is a permanent misconfiguration, so logging it per request would flood the @@ -67,10 +73,6 @@ $http_ip_forward_header = ''; } -// ----------------------------- -// DO NOT MODIFY BELOW THIS LINE -// ----------------------------- - // the HTTP response headers captured via fopen or curl $httpResponseHeaders = array(); diff --git a/tests/ProxyTest.php b/tests/ProxyTest.php index 6d9fe02..46ea610 100644 --- a/tests/ProxyTest.php +++ b/tests/ProxyTest.php @@ -1237,7 +1237,7 @@ public function test_visitor_ip_removal_ignores_client_ip_headers() { $headers = [ 'X-Forwarded-For' => '8.8.8.8', - 'Client-Ip' => '6.6.6.6', + 'Client-Ip' => '9.9.9.9', 'Cf-Connecting-Ip' => '6.6.6.6', 'X-Test-Remove-Visitor-Ip' => '1', ]; @@ -1258,6 +1258,7 @@ public function test_visitor_ip_removal_ignores_client_ip_headers() $this->assertEquals(200, $response->getStatusCode()); $this->assertEquals($expected, $responseBody); $this->assertStringNotContainsString('8.8.8.8', $responseBody); + $this->assertStringNotContainsString('9.9.9.9', $responseBody); $this->assertStringNotContainsString('6.6.6.6', $responseBody); } @@ -1488,6 +1489,105 @@ public function test_visitor_ip_removal_bulk_marker_at_offset_zero_gets_placehol $this->assertStringNotContainsString('127.0.0.1', $responseBody); } + public function test_visitor_ip_removal_forwards_undecodable_bulk_body_unchanged() + { + // Documented limitation: a body the proxy cannot decode is passed through as-is (Matomo + // cannot parse it either, so it tracks nothing). The proxy must still contribute no token. + $body = '{"requests":["?idsite=1&rec=1&cip=6.6.6.6"'; + + $response = $this->sendBulkWithoutVisitorIp($body); + + $responseBody = $this->getBody($response); + + $this->assertEquals(200, $response->getStatusCode()); + $this->assertStringContainsString('RAW: ' . $body, $responseBody); + $this->assertStringNotContainsString('', $responseBody); + $this->assertStringNotContainsString('8.8.8.8', $responseBody); + } + + public function test_visitor_ip_removal_leaves_bulk_entry_without_query_unchanged() + { + // No '?' means no query for either side's parse_url, so Matomo discards the entry. The proxy + // must forward it as-is rather than trying to rewrite something it cannot parse. + $body = '{"requests":["idsite=1&rec=1&cip=6.6.6.6"]}'; + + $response = $this->sendBulkWithoutVisitorIp($body); + + $responseBody = $this->getBody($response); + + $this->assertEquals(200, $response->getStatusCode()); + $this->assertStringContainsString('"idsite=1&rec=1&cip=6.6.6.6"', $responseBody); + $this->assertStringNotContainsString('0.0.0.0', $responseBody); + $this->assertStringNotContainsString('8.8.8.8', $responseBody); + } + + public function test_visitor_ip_removal_bulk_list_entry_gets_placeholder_alongside_its_values() + { + // A list-typed entry is an array to both sides, so Matomo reads its keys as params: our + // placeholder lands under 'cip' and the nested string stays an inert '0' param. + $body = '{"requests":[["?idsite=1&rec=1&cip=6.6.6.6"]]}'; + + $response = $this->sendBulkWithoutVisitorIp($body); + + $responseBody = $this->getBody($response); + + $this->assertEquals(200, $response->getStatusCode()); + $this->assertStringContainsString('{"0":"?idsite=1&rec=1&cip=6.6.6.6","cip":"0.0.0.0"}', $responseBody); + $this->assertStringNotContainsString('8.8.8.8', $responseBody); + } + + public function test_visitor_ip_removal_does_not_touch_post_body_on_opt_out_endpoint() + { + $response = $this->send( + 'module=CoreAdminHome&action=optOut&raw_input=1', + null, + null, + [ + 'content-type' => 'application/x-www-form-urlencoded', + 'X-Forwarded-For' => '8.8.8.8', + 'X-Test-Remove-Visitor-Ip' => '1', + ], + '/matomo-proxy.php', + 'POST', + 'cip=6.6.6.6&other=1' + ); + + $responseBody = $this->getBody($response); + + $this->assertEquals(200, $response->getStatusCode()); + // Not a tracking endpoint: the proxy contributes no cip and no token either way, and must + // not rewrite the client's body. + $this->assertStringContainsString('RAW: cip=6.6.6.6&other=1', $responseBody); + $this->assertStringNotContainsString('0.0.0.0', $responseBody); + $this->assertStringNotContainsString('', $responseBody); + $this->assertStringNotContainsString('8.8.8.8', $responseBody); + } + + public function test_visitor_ip_removal_does_not_touch_post_body_on_plugin_config_endpoint() + { + $response = $this->send( + 'idsite=35&raw_input=1', + null, + null, + [ + 'content-type' => 'application/x-www-form-urlencoded', + 'X-Forwarded-For' => '8.8.8.8', + 'X-Test-Remove-Visitor-Ip' => '1', + ], + '/plugins/HeatmapSessionRecording/configs.php', + 'POST', + 'cip=6.6.6.6&other=1' + ); + + $responseBody = $this->getBody($response); + + $this->assertEquals(200, $response->getStatusCode()); + $this->assertStringContainsString('RAW: cip=6.6.6.6&other=1', $responseBody); + $this->assertStringNotContainsString('0.0.0.0', $responseBody); + $this->assertStringNotContainsString('', $responseBody); + $this->assertStringNotContainsString('8.8.8.8', $responseBody); + } + public function test_visitor_ip_removal_takes_precedence_over_forward_header() { $headers = [ @@ -1635,7 +1735,13 @@ private function sendBulkWithoutVisitorIp($body) 'raw_input=1', null, null, - ['content-type' => 'application/x-www-form-urlencoded', 'X-Test-Remove-Visitor-Ip' => '1'], + [ + 'content-type' => 'application/x-www-form-urlencoded', + // A visitor IP getVisitIp() would pick up, so the assertions prove the header + // sources are bypassed and not merely that REMOTE_ADDR is unused. + 'X-Forwarded-For' => '8.8.8.8', + 'X-Test-Remove-Visitor-Ip' => '1', + ], null, 'POST', $body diff --git a/tests/server/matomo.php b/tests/server/matomo.php index e147bac..6893d6d 100644 --- a/tests/server/matomo.php +++ b/tests/server/matomo.php @@ -23,7 +23,7 @@ } $headers = array(); -foreach (array('DNT', 'X_DO_NOT_TRACK', 'X_FORWARDED_FOR', 'X_REAL_IP', 'CLIENT_IP', 'CF_CONNECTING_IP', 'COOKIE') as $headerName) { +foreach (array('DNT', 'X_DO_NOT_TRACK', 'X_FORWARDED_FOR', 'CLIENT_IP', 'CF_CONNECTING_IP', 'COOKIE') as $headerName) { if (isset($_SERVER['HTTP_' . $headerName])) { $headers[$headerName] = $_SERVER['HTTP_' . $headerName]; } From 70a8cba4d5e759030cc0e27c4d8a7ccb8bf22d3a Mon Sep 17 00:00:00 2001 From: sgiehl Date: Tue, 4 Aug 2026 14:58:50 +0200 Subject: [PATCH 4/5] Fix the error-log claim and restructure the visitor-IP removal docs The README said a warning is written to the error log for as long as $REMOVE_VISITOR_IP and $http_ip_forward_header are both configured, which stopped being true when that error_log() was gated on $DEBUG_PROXY. It also claimed setting the option was "the only setup step" immediately above two warnings about things to change in Matomo. Both descriptions had grown by accumulation, with the two facts most likely to take tracking down - the write-token requirement and the TrackingSpamPrevention limits - separated by around 25 lines of qualifying prose. Reordered to lead with what the option does and its one setup step, then those two checks, then the impact table, then the limits and edge cases as a scannable list. No claim changed except the error-log one; the rest was re-verified against the implementation and against Matomo core. Also notes that the guarantee covers what the proxy sends: anything the operator's own infrastructure inserts between the proxy and Matomo, such as a reverse proxy or WAF adding X-Forwarded-For, is outside its control. That was the one remaining route for an IP to arrive and had never been written down. Says a request keeps its own cip and receives no token from us, rather than that it is "forwarded untouched" - a single request's query is re-encoded through http_build_query(), so only the values are preserved, not the bytes. The empty/array-cip replacement is no longer stated as unconditional either: the decision is made across $_GET and $_POST together, so a non-empty cip in either one leaves both alone. config.php.example follows the same order and loses the note about which truthy values enable the flag, which described PHP rather than the feature. Test changes alongside: the no-query bulk entry test now asserts that the batch still receives a batch-level token, which authorizes nothing because Matomo drops such an entry before building a request from it; the matomo.js test states what it cannot cover, since the fake matomo.js is a static file and cannot echo the headers the proxy sent; and a new test pins the case where a client's own token_auth authorizes the placeholder we injected, which is the security-relevant intersection of the two features. --- README.md | 34 ++++++++++++++++------------------ config.php.example | 28 +++++++++++----------------- tests/ProxyTest.php | 38 +++++++++++++++++++++++++++++++++++++- 3 files changed, 64 insertions(+), 36 deletions(-) diff --git a/README.md b/README.md index b5cf4d9..54cc134 100644 --- a/README.md +++ b/README.md @@ -126,29 +126,20 @@ Because the proxy sits between your visitors and Matomo, it has to tell Matomo t > ⚠️ **Behavior change:** two cases that previously ended up with no `cip` at all — so Matomo recorded the proxy's IP instead of the visitor's — now send the visitor IP as normal. This applies whether or not `$REMOVE_VISITOR_IP` is set: > -> - a request whose `cip` is empty or array-valued, which Matomo ignores in favour of the connection IP, so the proxy now treats it as absent; +> - a request whose only `cip` is empty or array-valued, which Matomo ignores in favour of the connection IP, so the proxy now treats it as absent; > - a `POST` body in which the bulk marker `"requests"` appears at the very start, which Matomo does not treat as a bulk request — the proxy now agrees with it instead of forwarding the body unprocessed. ### Removing the visitor IP -Some organisations are required to ensure the visitor IP never reaches the analytics application at all — not even to be anonymised there. Matomo's own IP anonymisation runs inside Matomo, so the full IP arrives there first. Setting `$REMOVE_VISITOR_IP = true;` in `config.php` moves that boundary out to the proxy. +Set `$REMOVE_VISITOR_IP = true;` in `config.php` to stop sending the visitor IP to Matomo altogether. The proxy forwards `cip=0.0.0.0` in its place — for single and bulk tracking requests alike — and never reads the visitor IP at all, so it cannot leak into a header either. -That is the only setup step — nothing needs to change in Matomo. With the option enabled: +Use this where the visitor IP must not reach Matomo even to be anonymised there, since Matomo's own IP anonymisation runs inside Matomo and the full IP would otherwise arrive first. -- the proxy sends `cip=0.0.0.0` instead of the visitor IP, for single and bulk tracking requests alike -- the proxy never reads the visitor IP at all, so it cannot leak it into a header either -- `$http_ip_forward_header` is ignored, since it would send the IP straight back, and a warning is written to the PHP error log for as long as both are configured — clear `$http_ip_forward_header` to stop it +Two things to check before you enable it: -> ⚠️ **The write/admin `$TOKEN_AUTH` is still required, and this includes deployments that previously used header-only mode without one.** Matomo only honors `cip` on an authenticated request, and otherwise **records nothing at all** — so removing or downgrading the token discards your data rather than degrading it. Watch out for how that surfaces: a single request is rejected with HTTP 400, but a **bulk** request (what the JavaScript tracker sends by default) comes back as HTTP 200 with `{"status":"success","tracked":0,"invalid":N}`, so there is no error status to alert on. +> ⚠️ **You still need the write/admin `$TOKEN_AUTH`** — including if you previously ran header-only mode without one. Matomo honors `cip` only on an authenticated request and otherwise records **nothing at all**, so a missing or read-only token discards your traffic instead of degrading it. This is easy to miss: a single request is rejected with HTTP 400, but a bulk request — what the JavaScript tracker sends by default — comes back as HTTP 200 with `{"status":"success","tracked":0,"invalid":N}`. -> Note: the placeholder is `0.0.0.0` rather than no `cip` at all because Matomo falls back to the IP of the connection whenever `cip` is empty. Sending nothing would make Matomo record the proxy's IP and report the proxy's location as though it were real visitor data. - -**What this option does not do.** It governs the IP *the proxy contributes*. A request that supplies its own `cip` is forwarded untouched, because Matomo only honors a `cip` on an authenticated request — so such a request is a deliberate decision to track a specific IP, made by something holding a valid token. "Supplies its own `cip`" means a non-empty one: an empty or array-valued `cip` is something Matomo ignores in favour of the connection IP, so the proxy replaces it with the placeholder rather than letting its own IP be recorded. The browser JavaScript tracker never sends `cip` at all, so ordinary visitor traffic is unaffected by this distinction. Two consequences worth knowing: - -- If you also want to forbid server-side integrations from submitting IPs, that belongs in those integrations, or in which tokens you issue — the proxy will not overrule them. -- This assumes Matomo's `tracking_requests_require_authentication` is at its default of `1`. If it has been set to `0`, Matomo honors an unauthenticated `cip`, and then anything could submit an IP. - -Explicit location parameters (`lat`, `long`, `city`, `region`, `country`) are likewise not removed — Matomo already requires authentication for those. For cookies that might embed an IP, see [Cookie forwarding](#cookie-forwarding) below; a `Cookie` header is forwarded as-is unless you set `$COOKIE_ALLOWLIST`. +> ⚠️ **Check TrackingSpamPrevention if you use its limits.** Every visit now reports `0.0.0.0`, so the first visitor to reach its *maximum actions per visit* limit gets `0.0.0.0/32` added to its blocked ranges — after which **every** visit is excluded and tracking stops entirely, silently. Its *included countries* setting behaves the same way, because the country now comes from Matomo's `Accept-Language` guess and is `xx` when it cannot be determined. Both are unset by default. If you use them, add `0.0.0.0` to `iprange_allowlist[]` under `[TrackingSpamPrevention]` in Matomo's `config.ini.php`, add `xx` to the country list, or leave the limits unset. #### Impact on your Matomo reports @@ -163,9 +154,16 @@ Explicit location parameters (`lat`, `long`, `city`, `region`, `country`) are li | Goals, funnels, ecommerce attribution | Affected only where the above merges separate visitors | | QueuedTracking | Requests without a visitor ID are sharded by IP, so they all land in one queue instead of being spread across the configured number | -> ⚠️ **TrackingSpamPrevention needs checking before you enable this.** Because every visit now reports `0.0.0.0`, the first visitor to *reach* the plugin's *maximum actions per visit* limit causes `0.0.0.0/32` to be added to its blocked IP ranges — after which **every** visit is excluded and tracking stops entirely, silently. That setting is unlimited by default, so this only bites if it has been changed; if you use it, either unset it or add `0.0.0.0` to `iprange_allowlist[]` under `[TrackingSpamPrevention]` in Matomo's `config.ini.php`. -> -> The same plugin's *excluded/included countries* settings are affected too: with no usable IP, the country comes from Matomo's `Accept-Language` guess and is `xx` when it cannot be determined. With *included countries* configured and `xx` not among them, every such visit is excluded — the same silent stoppage. +#### Limits and edge cases + +- **The option covers the IP the proxy contributes, not one a caller sends deliberately.** A request supplying its own non-empty `cip` keeps it — the proxy adds neither a `cip` nor a token of its own — because Matomo honors `cip` only for a valid token holder, making such a request a deliberate decision to track a specific IP. The JavaScript tracker never sends `cip`, so ordinary visitor traffic is unaffected. To rule that out too, change those integrations or which tokens you issue; the proxy will not overrule them. +- An **empty or array-valued `cip`** is not such a decision — Matomo ignores those in favour of the connection IP — so the proxy drops it and sends the placeholder instead. The query string and the POST body are judged together: if a non-empty `cip` appears in either, the request counts as deliberate and neither is touched. +- **Explicit location parameters** (`lat`, `long`, `city`, `region`, `country`) are not removed either, since Matomo already requires authentication for them. +- **Cookies** are forwarded unchanged unless you set `$COOKIE_ALLOWLIST` — see [Cookie forwarding](#cookie-forwarding) below. +- **`$http_ip_forward_header` is ignored** while this is on, since it would send the IP straight back. Clear it to remove the conflict; with `$DEBUG_PROXY` enabled the proxy also notes the conflict in the PHP error log. +- **The guarantee covers what the proxy sends.** Anything your own infrastructure adds to the outbound request between the proxy and Matomo — a reverse proxy, WAF or egress proxy inserting `X-Forwarded-For`, for example — is outside the proxy's control and needs checking separately. +- This assumes Matomo's **`tracking_requests_require_authentication`** is at its default of `1`. Set to `0`, Matomo honors an unauthenticated `cip`, so anything could submit an IP. +- The placeholder is `0.0.0.0` rather than **no `cip` at all** because Matomo falls back to the connection IP whenever `cip` is empty — sending nothing would record the proxy's own IP and report its location as real visitor data. ### Cookie forwarding diff --git a/config.php.example b/config.php.example index efc39da..4edb519 100644 --- a/config.php.example +++ b/config.php.example @@ -50,25 +50,19 @@ $user_agent = ''; // $http_ip_forward_header = ''; -// Set this to true to stop sending the visitor IP to Matomo altogether. The proxy then forwards the -// placeholder 0.0.0.0 as `cip` instead of the real IP, and $http_ip_forward_header above is ignored. -// Intended for deployments where the visitor IP must not reach Matomo at all, even before -// anonymisation. Any non-empty value other than '0' enables it. +// Set this to true to stop sending the visitor IP to Matomo altogether: the proxy forwards the +// placeholder 0.0.0.0 as `cip` instead, and $http_ip_forward_header above is ignored. Use it where +// the visitor IP must not reach Matomo at all, not even to be anonymised there. // -// Your $TOKEN_AUTH is still required: Matomo records nothing at all for an unauthenticated `cip`, -// so removing the token discards your traffic rather than degrading it. A single request is rejected -// with HTTP 400, but a bulk request - what the JS tracker sends by default - returns HTTP 200 with -// "tracked":0, so there is no error status to alert on. +// Your $TOKEN_AUTH is still required. Matomo honors `cip` only on an authenticated request and +// otherwise records nothing at all, so a missing or read-only token discards your traffic instead of +// degrading it - and quietly: a single request is rejected with HTTP 400, but a bulk request (what +// the JS tracker sends by default) returns HTTP 200 with "tracked":0. // -// This governs the IP the proxy itself contributes. A request supplying its own non-empty `cip` is -// left untouched, because Matomo only honors one for an authenticated request - so it is a -// deliberate choice to track a specific IP. Location parameters a client supplies explicitly (lat, -// long, city, region, country) are likewise not removed. -// -// Impacts: Location and Provider reports become "Unknown", the visitor IP shows as 0.0.0.0, and -// IP-based exclusions and bot blocking can no longer identify individual visitors. Visit counts also -// become less accurate for visitors without cookies. See README.md for the full list and for an -// important warning about the TrackingSpamPrevention plugin. +// Location and Provider reports then read "Unknown" and every visit shows 0.0.0.0. Read the impact +// list in README.md before enabling this: it also covers a TrackingSpamPrevention setting that can +// stop tracking entirely, and which requests keep their own values (one supplying its own `cip`, and +// explicit lat/long/city/region/country parameters). $REMOVE_VISITOR_IP = false; // By default, the proxy forwards the visitor's entire Cookie header to Matomo unchanged, which diff --git a/tests/ProxyTest.php b/tests/ProxyTest.php index 46ea610..7ab982c 100644 --- a/tests/ProxyTest.php +++ b/tests/ProxyTest.php @@ -1367,6 +1367,32 @@ public function test_visitor_ip_removal_bulk_replaces_empty_client_cip_with_plac $this->assertStringContainsString('"token_auth":""', $responseBody); } + public function test_visitor_ip_removal_lets_a_client_token_authorize_the_placeholder() + { + $headers = [ + 'X-Forwarded-For' => '8.8.8.8', + 'X-Test-Remove-Visitor-Ip' => '1', + ]; + $response = $this->send('idsite=1&token_auth=client-token', null, null, $headers); + + $responseBody = $this->getBody($response); + + // The client authenticates, so the proxy withholds its own token - the placeholder it added + // is authorized by the client's token instead. + $expected = << '0.0.0.0', + 'idsite' => '1', + 'token_auth' => 'client-token', +) +RESPONSE; + + $this->assertEquals(200, $response->getStatusCode()); + $this->assertEquals($expected, $responseBody); + $this->assertStringNotContainsString('', $responseBody); + $this->assertStringNotContainsString('8.8.8.8', $responseBody); + } + public function test_visitor_ip_removal_keeps_explicit_location_params() { $response = $this->send('idsite=1&lat=1&long=2', null, null, ['X-Test-Remove-Visitor-Ip' => '1']); @@ -1519,6 +1545,9 @@ public function test_visitor_ip_removal_leaves_bulk_entry_without_query_unchange $this->assertStringContainsString('"idsite=1&rec=1&cip=6.6.6.6"', $responseBody); $this->assertStringNotContainsString('0.0.0.0', $responseBody); $this->assertStringNotContainsString('8.8.8.8', $responseBody); + // The batch still counts as clean, so it gets a batch-level token. That authorizes nothing + // here: Matomo drops an entry it cannot parse into params before building a request from it. + $this->assertStringContainsString('"token_auth":""', $responseBody); } public function test_visitor_ip_removal_bulk_list_entry_gets_placeholder_alongside_its_values() @@ -1687,10 +1716,17 @@ public function test_visitor_ip_removal_sends_no_ip_header_on_opt_out_endpoint() public function test_visitor_ip_removal_still_serves_matomo_js() { - $response = $this->send(null, null, null, ['X-Test-Remove-Visitor-Ip' => '1']); + // Smoke test only: the fake matomo.js is a static file, so it cannot echo the headers the + // proxy sent. That the option suppresses the IP-forward header on non-tracking requests is + // covered by test_visitor_ip_removal_sends_no_ip_header_on_{plugin_config,opt_out}_endpoint. + $response = $this->send(null, null, null, [ + 'X-Test-Ip-Forward-Header' => 'X-Forwarded-For', + 'X-Test-Remove-Visitor-Ip' => '1', + ]); $this->assertEquals(200, $response->getStatusCode()); $this->assertEquals('this is matomo.js', $response->getBody()->getContents()); + $this->assertEquals('application/javascript; charset=UTF-8', $response->getHeader('Content-Type')[0]); } public function test_falsy_remove_visitor_ip_keeps_forwarding_the_visitor_ip() From e8cf27e77b0cdad7b1dad4e63702f28c180663f9 Mon Sep 17 00:00:00 2001 From: sgiehl Date: Wed, 5 Aug 2026 18:26:31 +0200 Subject: [PATCH 5/5] Resolve the client cip the way Matomo does before judging it Matomo builds its tracker params as $_GET + $_POST (Tracker\RequestSet), so a cip key in the query wins over one in the body whatever its value. The proxy asked clientSuppliesVisitIp() about the two arrays separately and skipped the block if either said yes, so a non-empty body cip could hide an empty query cip that Matomo would actually read: POST /matomo.php?idsite=1&cip= body: cip=6.6.6.6 Both were forwarded untouched, Matomo resolved cip to '' and fell back to the connection IP, and the visit was recorded against the proxy - the outcome the empty-cip handling exists to prevent. No visitor IP leaked, but the fix did not hold for that shape. Judging $_GET + $_POST mirrors the expression Matomo itself uses, so the two cannot disagree. Verified against the four relevant shapes: an empty or array query cip now gets the placeholder even when the body carries a non-empty one, while a body cip with no query cip is still honoured untouched. clientProvidesAuthParams() needs no matching change: the unset above it drops both cip entries first, so the token decision no longer sees them. Reported by tzi in review. --- README.md | 2 +- proxy.php | 6 ++- tests/ProxyTest.php | 107 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 113 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 54cc134..122ac6e 100644 --- a/README.md +++ b/README.md @@ -157,7 +157,7 @@ Two things to check before you enable it: #### Limits and edge cases - **The option covers the IP the proxy contributes, not one a caller sends deliberately.** A request supplying its own non-empty `cip` keeps it — the proxy adds neither a `cip` nor a token of its own — because Matomo honors `cip` only for a valid token holder, making such a request a deliberate decision to track a specific IP. The JavaScript tracker never sends `cip`, so ordinary visitor traffic is unaffected. To rule that out too, change those integrations or which tokens you issue; the proxy will not overrule them. -- An **empty or array-valued `cip`** is not such a decision — Matomo ignores those in favour of the connection IP — so the proxy drops it and sends the placeholder instead. The query string and the POST body are judged together: if a non-empty `cip` appears in either, the request counts as deliberate and neither is touched. +- An **empty or array-valued `cip`** is not such a decision — Matomo ignores those in favour of the connection IP — so the proxy drops it and sends the placeholder instead. Matomo resolves `cip` from the query string and the POST body together, with the query winning, and the proxy judges that same effective value. - **Explicit location parameters** (`lat`, `long`, `city`, `region`, `country`) are not removed either, since Matomo already requires authentication for them. - **Cookies** are forwarded unchanged unless you set `$COOKIE_ALLOWLIST` — see [Cookie forwarding](#cookie-forwarding) below. - **`$http_ip_forward_header` is ignored** while this is on, since it would send the IP straight back. Clear it to remove the conflict; with `$DEBUG_PROXY` enabled the proxy also notes the conflict in the PHP error log. diff --git a/proxy.php b/proxy.php index 8995b5a..27f28fa 100644 --- a/proxy.php +++ b/proxy.php @@ -150,7 +150,11 @@ // The batch token now lives in the JSON body; never also send one in the forwarded query. unset($_GET['token_auth']); } else { - if (!clientSuppliesVisitIp($_GET) && !clientSuppliesVisitIp($_POST)) { + // Judge the same cip Matomo will read: it resolves tracker params as $_GET + $_POST + // (Tracker\RequestSet), so a cip key in the query wins over one in the body whatever + // its value. Checking the two separately would let an empty query cip hide behind a + // non-empty body cip that Matomo never reads. + if (!clientSuppliesVisitIp($_GET + $_POST)) { // Drop an empty/array cip, which Matomo ignores anyway, so it can't clobber ours // when $_GET is merged below (array_merge lets $_GET win on key collision). unset($_GET['cip'], $_POST['cip']); diff --git a/tests/ProxyTest.php b/tests/ProxyTest.php index 7ab982c..8829c3e 100644 --- a/tests/ProxyTest.php +++ b/tests/ProxyTest.php @@ -192,6 +192,35 @@ public function test_plugin_config_php_proxied_correctly() $this->assertEquals($expected, $responseBody); } + public function test_empty_query_cip_wins_over_a_body_cip_and_is_replaced() + { + // Matomo resolves tracker params as $_GET + $_POST, so the empty query cip is the one it + // would read - the non-empty body cip never reaches it and must not suppress our injection. + $response = $this->send( + 'idsite=1&cip=', + null, + null, + ['content-type' => 'application/x-www-form-urlencoded'], + null, + 'POST', + 'cip=6.6.6.6' + ); + + $responseBody = $this->getBody($response); + + $expected = << '127.0.0.1', + 'token_auth' => '', + 'idsite' => '1', +) +RESPONSE; + + $this->assertEquals(200, $response->getStatusCode()); + $this->assertEquals($expected, $responseBody); + $this->assertStringNotContainsString('6.6.6.6', $responseBody); + } + public function test_post_requests_are_proxied_correctly() { $response = $this->send('foo=bar', null, null, ['content-type' => 'application/x-www-form-urlencoded'], null, 'POST', 'baz=buz'); @@ -1352,6 +1381,84 @@ public function test_visitor_ip_removal_replaces_array_client_cip_with_placehold $this->assertStringNotContainsString('6.6.6.6', $responseBody); } + public function test_visitor_ip_removal_replaces_empty_query_cip_that_wins_over_a_body_cip() + { + $headers = [ + 'content-type' => 'application/x-www-form-urlencoded', + 'X-Forwarded-For' => '8.8.8.8', + 'X-Test-Remove-Visitor-Ip' => '1', + ]; + $response = $this->send('idsite=1&cip=', null, null, $headers, null, 'POST', 'cip=6.6.6.6'); + + $responseBody = $this->getBody($response); + + // Matomo would read the empty query cip and fall back to the connection IP - the proxy's - + // so judging the query and body separately would let the body cip hide the gap. + $expected = << '0.0.0.0', + 'token_auth' => '', + 'idsite' => '1', +) +RESPONSE; + + $this->assertEquals(200, $response->getStatusCode()); + $this->assertEquals($expected, $responseBody); + $this->assertStringNotContainsString('6.6.6.6', $responseBody); + $this->assertStringNotContainsString('8.8.8.8', $responseBody); + } + + public function test_visitor_ip_removal_replaces_array_query_cip_that_wins_over_a_body_cip() + { + $headers = [ + 'content-type' => 'application/x-www-form-urlencoded', + 'X-Forwarded-For' => '8.8.8.8', + 'X-Test-Remove-Visitor-Ip' => '1', + ]; + $response = $this->send('idsite=1&cip[]=1.2.3.4', null, null, $headers, null, 'POST', 'cip=6.6.6.6'); + + $responseBody = $this->getBody($response); + + $expected = << '0.0.0.0', + 'token_auth' => '', + 'idsite' => '1', +) +RESPONSE; + + $this->assertEquals(200, $response->getStatusCode()); + $this->assertEquals($expected, $responseBody); + $this->assertStringNotContainsString('1.2.3.4', $responseBody); + $this->assertStringNotContainsString('6.6.6.6', $responseBody); + } + + public function test_visitor_ip_removal_keeps_a_body_cip_when_the_query_has_none() + { + $headers = [ + 'content-type' => 'application/x-www-form-urlencoded', + 'X-Test-Remove-Visitor-Ip' => '1', + ]; + $response = $this->send('idsite=1', null, null, $headers, null, 'POST', 'cip=6.6.6.6'); + + $responseBody = $this->getBody($response); + + // With no cip in the query, the body cip is the one Matomo reads, so it counts as deliberate. + $expected = << '1', +) +array ( + 'cip' => '6.6.6.6', +) +RESPONSE; + + $this->assertEquals(200, $response->getStatusCode()); + $this->assertEquals($expected, $responseBody); + $this->assertStringNotContainsString('0.0.0.0', $responseBody); + $this->assertStringNotContainsString('', $responseBody); + } + public function test_visitor_ip_removal_bulk_replaces_empty_client_cip_with_placeholder() { $body = '{"requests":["?idsite=1&rec=1&action_name=one&cip="]}';