Add index.php to proxy opt out HTTP requests + fix regressions in curl support. #37

diosmosis · 2018-04-26T02:49:42Z

Changes:

Add matomo-proxy.php endpoint that only allows proxying the optout method.
Return correct value in handleHeaderLine() to avoid obscure curl error.
The curl HTTP header option requires an array not a string, so give it one.
Forward the Cookie header.
Tests for index.php.
Blurb in readme about proxying the opt-out form.

Tested w/ fopen & curl.

Fixes #29

…l support.

…cker-proxy in their root folder.

Findus23

Hi, I tested it locally and everything works file.
I left two comments with minor nitpicks.

One more thing that might break (or I am misunderstanding something):
Let's say example.com gets tracked by matomo.example.
While normally the optout cookie is just set to matomo.example, with the proxy, the opt-out will be at example.com.
So will it then set the cookie to example.com?
If no, I guess it will fail as it tries to set a cookie to a foreign domain (I guess)
If yes, does Matomo know to handle the opt-out cookie if it is on another domain as itself?

I guess it should work, but I am not completely sure

Findus23 · 2018-04-26T14:05:29Z

proxy.php

-        'timeout'    => $timeout
+        'timeout'    => $timeout,
+
+        // 'ignore_errors' => true, // uncomment to debug error code responses


Is it possible that this is the wrong way around? If commenting is for debugging, it shouldn't be uncommitted by default.

It’s commented so developers don’t have to look it up next time when there is an error and zero output.

Findus23 · 2018-04-26T14:12:59Z

matomo-proxy.php

+$allParams = $_GET + $_POST;
+
+if (!in_array("{$allParams['module']}.{$allParams['action']}", $SUPPORTED_METHODS)) {
+    http_response_code(404);


After trying around a bit I couldn't find a way to circumvent this check.
But I noticed that a simple request to matomo-proxy.php causes an error (because module and action are not set), which is a bit ugly.

And the whole security depends on the order of $_GET + $_POST and the next person to edit this to know that php adds arrays from right to left overwriting POST by GET.

If someone switches this around accidentally, all Matomo modules can be accessed:

curl --data "module=CoreAdminHome&action=optOut" "http://localhost/tracker-proxy/matomo-proxy.php?module=Login&idSite=1&period=day&date=yesterday&action=

So I'd at least add a comment so it is clear that this is important. (at least for me it wasn't clear otherwise)

diosmosis · 2018-04-26T18:46:45Z

Re cookies:

The proxy will just forward the set-cookie header, so the cookies will be associated w/ the tracker-proxy domain. So as long as tracking is done through the tracker-proxy domain, it will receive the ignore cookie. (Tested locally via /etc/hosts).

Also checked to see what will happen if the cookie_domain INI config option is set, and it doesn't apply to the ignore cookie (it's never created w/ a domain).

… module/action & empty module/action.

diosmosis · 2018-04-26T19:00:00Z

Updated to check $_GET/$_POST w/o merging the arrays + check for absent module/action.

level420 · 2018-05-02T13:48:21Z

@diosmosis @mattab see my comments in #29

There are 2 problems:

If the proxy is communicating with matomo instance which is secured (https://) from an unsecured site (http://), matomo tries to set the PIWIK_SESSID cookie for the secured site which is not available for the unsecured one. The iframe is displayed correctly, but clicking on the checkbox does not set the piwik_ignore cookie.
The opt-out iframe only works if it is able to set the PIWIK_SESSID cookie, which is in my case undesired as I'd like to work without any matomo cookies, besides of the piwik_ignore cookie.
Opt out should also be possible with a simple cookie piwik_ignore with value ignore or similar, so we wouldn't need any special value for the opt out to work. We could also ommit the complete opt out proxy as a simple php file is sufficient which sets or deletes the cookie for the current domain.

…d $DEBUG_PROXY global that can be set during development.

diosmosis · 2018-05-02T21:45:08Z

Updated to support http proxy => https matomo.

mattab · 2018-05-03T11:34:01Z

Opt out should also be possible with a simple cookie piwik_ignore with value ignore or similar, so we wouldn't need any special value for the opt out to work. We could also ommit the complete opt out proxy as a simple php file is sufficient which sets or deletes the cookie for the current domain.

@level420 That's a good idea I think, but this would be a core change and improved in the opt-out iframe I believe. so maybe you can create an issue for this idea in https://github.com/matomo-org/matomo/issues ?

tsteur · 2018-05-08T19:51:58Z

proxy.php

@@ -11,6 +11,8 @@
    exit; // this file is not supposed to be accessed directly
 }

+$DEBUG_PROXY = false;


Should it be more like $NO_VERIFY_SSL or maybe better IGNORE_HTTP_ERRORS? or something? DEBUG_PROXY is good though as well and maybe best 👍 . Would maybe just explain what it does in a comment

It does a couple other things for debugging. I can split it into DEBUG_PROXY and NO_VERIFY_SSL (DEBUG_PROXY also removes error silencing so file_get_contents will spit out error messages).

👍 if splitted, it would actually allow someone to use it with a self signed certificate etc without silencing other errors 👍

tsteur · 2018-05-08T19:53:08Z

proxy.php

@@ -260,6 +299,14 @@ function getHttpContentAndStatus($url, $timeout, $user_agent)
            curl_setopt($ch, CURLOPT_POSTFIELDS, $stream_options['http']['content']);
        }

+        if (isset($stream_options['ssl']['verify_peer']) && $stream_options['ssl']['verify_peer'] == false) {


is this the same as using DEBUG_PROXY in the if?

No, that's done above, this translates the stream options to curl options.

Not sure I'm getting it. By the looks they are only set to false when using $DEBUG_PROXY = true correct? I'm thinking instead of translating it might be easier to read by using if($DEBUG_PROXY). May be also less fragile if eg for some reason the stream option changes it doesn't change the curl behaviour etc. But I didn't have a deep look I might be missing something.

The existing code follows the pattern of:

set stream_options for file_get_contents

if curl, transform already set stream options to curl options

The stream option name won't change, it's the actual php option name. It might change in a PHP version, but that's different than a developer deciding to rename it (which they can't do).

diosmosis added the not-in-changelog label Apr 26, 2018

Add index.php to proxy opt out HTTP requests + fix regressions in cur…

d41a4ca

…l support.

diosmosis force-pushed the opt-out branch from ea010aa to d41a4ca Compare April 26, 2018 02:51

diosmosis and others added 5 commits April 25, 2018 19:54

Rename index.php to matomo-index.php to support users who install tra…

6ff4144

…cker-proxy in their root folder.

Add section to readme for opt-out.

7a8c948

Rename endpoint to matomo-proxy.php.

41bb17d

Fix tests.

6b8f8c6

Privacy pages structure has changed in 3.5.0

4bf1e74

mattab mentioned this pull request Apr 26, 2018

Support Opt-Out Cookie and forward Opt-Out iFrame #29

Closed

Findus23 requested changes Apr 26, 2018

View reviewed changes

Check $_GET/$_POST seperately in index.php & add tests for wrong post…

3197e50

… module/action & empty module/action.

diosmosis added 2 commits May 2, 2018 14:17

Finish docs in config.php.example.

08e9906

If proxy is on HTTP then do not serve secure cookies from matomo + ad…

6044e5b

…d $DEBUG_PROXY global that can be set during development.

diosmosis added this to the Current sprint milestone May 3, 2018

diosmosis added 2 commits May 7, 2018 21:52

Merge branch 'master' into opt-out

e095242

Merge branch 'master' into opt-out

3c62600

tsteur reviewed May 8, 2018

View reviewed changes

Split $DEBUG_PROXY into $DEBUG_PROXY & $NO_VERIFY_SSL.

6c0bc7d

tsteur mentioned this pull request May 9, 2018

On opt-out page move js to separate file to fix inline-script (CSP) matomo-org/matomo#12873

Merged

mattab merged commit 4e78b41 into master May 12, 2018

mattab deleted the opt-out branch May 12, 2018 01:10

innocraft-automation removed this from the Current sprint milestone Jan 24, 2023

Add index.php to proxy opt out HTTP requests + fix regressions in curl support. #37

Add index.php to proxy opt out HTTP requests + fix regressions in curl support. #37

Uh oh!

Conversation

diosmosis commented Apr 26, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Findus23 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

diosmosis commented Apr 26, 2018

Uh oh!

diosmosis commented Apr 26, 2018

Uh oh!

level420 commented May 2, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

diosmosis commented May 2, 2018

Uh oh!

mattab commented May 3, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

diosmosis commented Apr 26, 2018 •

edited

Loading

level420 commented May 2, 2018 •

edited

Loading