added proxy feature for Matomo Tag Manager #79
Conversation
|
Hello! Do you have any information when the solution will be ready? |
The following tasks are as of 2023-02-07 open: Security Review doneAs 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 doneOf 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/possibleNeed to add these, sorry for the delay, just noticed https://github.com/matomo-org/tracker-proxy#running-the-tests Developer changelog updated if neededNo update in main matomo needed, found no changelog in this repo. Documentation added if neededNo update in main matomo needed, added documentation to be in line with other documentation found in this repo. Existing documentation updated if neededSee 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. |
|
@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? |
|
@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. |
|
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 : 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 :
|
|
A user has asked "when will this feature be ready?":
|
|
how do i use it?? any plan on merging it |
|
@tobihille can you please explain how to use it |
|
Had another request for Tag Manager support for this today |
|
Same here, had a few clients asking for this. |
@wrtz008 Nope, I can see if I can allot sometime next week to test this. |
super appreciated @AltamashShaikh thank you for taking the time on this 🙏 |
| // with your Matomo URL ending with a slash. | ||
| // This URL will never be revealed to visitors or search engines. | ||
| if (! isset($MATOMO_URL)) { | ||
| $MATOMO_URL = 'http://your-matomo-domain.example.org/matomo/'; |
There was a problem hiding this comment.
$MATOMO_URL should be used from config, if not set simply do not do anything.
| // Edit the line below and replace http://your-tracker-proxy.org/ with the URL to your tracker-proxy | ||
| // setup. This URL will be used in Matomo output that contains the Matomo URL, so your Matomo is effectively | ||
| // hidden. | ||
| if (! isset($PROXY_URL)) { |
There was a problem hiding this comment.
same here this should also be part of config
| } | ||
|
|
||
| $fileName = $MATOMO_URL . $calledJsFile; | ||
| $fileContent = file_get_contents($fileName); |
There was a problem hiding this comment.
check file exists too ?
| @@ -0,0 +1,3 @@ | |||
| RewriteEngine On | |||
|
|
|||
| RewriteRule ^(.*)\.js$ index.php | |||
There was a problem hiding this comment.
can't we add a rule for a specific js ?
AltamashShaikh
left a comment
AltamashShaikh
left a comment
There was a problem hiding this comment.
left few comments
10 participants
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