-
Notifications
You must be signed in to change notification settings - Fork 46
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
added proxy feature for Matomo Tag Manager #79
base: master
Are you sure you want to change the base?
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 |
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