matomo-org / tracker-proxy

HTTP proxy for Matomo's tracker API. This script allows to track websites with Matomo without revealing to your visitors the secret Matomo server URL.
http://matomo.org/faq/how-to/faq_132/
GNU General Public License v3.0
152 stars 46 forks source link

added proxy feature for Matomo Tag Manager #79

Open tobihille opened 2 years ago

tobihille commented 2 years ago

Description:

This should fix #59, at least on my setup it is working in production.

I tried to get as much inspiration as possible from proxy.php and the other config values present in config.php.example while also only using older functions (no str_contains) to make it as compatible as possible.

Review

Codov commented 1 year ago

Hello! Do you have any information when the solution will be ready? Thank you!

tobihille commented 1 year ago

Hello! Do you have any information when the solution will be ready? Thank you!

The following tasks are as of 2023-02-07 open:

Security Review done

As already stated in my original message: I am no security expert, someone other than me needs to take a look at this. I know enough about security to know to leave tasks like this to experts.

Code review done

Of course I reviewed my own code before contributing but in my opinion this is also a task of e.g. a maintainer or a contributor.

Tests were added if useful/possible

Need to add these, sorry for the delay, just noticed https://github.com/matomo-org/tracker-proxy#running-the-tests

Developer changelog updated if needed

No update in main matomo needed, found no changelog in this repo.

Documentation added if needed

No update in main matomo needed, added documentation to be in line with other documentation found in this repo.

Existing documentation updated if needed

See above point.


I would appreciate help regarding the unit tests as I just noticed these here. But the changes are still in production on my side and no errors have been found.

sgiehl commented 1 year ago

@AltamashShaikh @snake14 Does anyone of you maybe has already worked with the tracker proxy and can check if this PR would be fine to merge?

snake14 commented 1 year ago

@sgiehl I don't recognise or have any experience with this plugin. The changes look alright, but I don't have any context and could be missing something.

thib3113 commented 1 year ago

Hello, the problem I saw reading this code, is that it's based on a htaccess ... So, it will only works with apache (and with htaccess enabled)

If I correctly understand how Tag manager works, it will use the js file name like an id ? so why not calling the "js" file with this id like : js/index.php?i=mqdmlqdmlqksd ? and in your script changing this to the filename ?

Also, I will need to do tests, but what about headers that are returned from matomo ? And what about the headers sent to the proxy ?

About security, you do "pretty" nothing . So, I didn't see lot of problems . But maybe :

atom-box commented 1 year ago

A user has asked "when will this feature be ready?":

"I would like the Tag Manager script not to report the domain of the analytics server but that of the monitored site."