@Skywalker-11 opened this Pull Request on May 9th 2018 Contributor

This PR moves the javascript part of the opt-out page that can be loaded via an iframe to a separate file. This allows the usage of CSP without the need to allow inline scripts or using hashes

@tsteur commented on May 9th 2018 Member

FYI: Merging this PR would break https://github.com/matomo-org/tracker-proxy/pull/37 which we finished yesterday cc @diosmosis

@Skywalker-11 commented on May 14th 2018 Contributor

This PR should now also work with the patched tracker-proxy which PR is referenced above. To accieve this an additional get parameter (matomoproxy) is added on the proxy. If it doesn't exist in the request of the opt-out iframe the normal path plugins/CoreAdminHome/javascripts/optOut.js is used.
If the parameter matomoproxy exists it will use ?module=CoreAdminHome&action=getOptOutJs as path for the script which is translated by the proxy to the normal path which then will be returned.

I'm not that familiar with the matomo routing so if there is a better way to do this please tell me.

@diosmosis commented on May 17th 2018 Member

I don't think using a special URL that doesn't actually work in matomo (ie, the ?module=CoreAdminHome&action=getOptOutJs) as a sort of message passing between matomo and the tracker-proxy is a good idea. It's not really clear from looking at the matomo code that this is what that special URL is for and it couples two entire repos together.

This should ideally be handled entirely in the tracker-proxy (so matomo doesn't have to know the tracker-proxy exists). Something like, in the tracker-proxy for matomo-proxy.php, rewriting URLs to JS/CSS files in the output to point to matomo-proxy.php instead (eg, tracker-proxy/matomo-proxy.php?file=plugins/CoreAdminHome/javascripts/optOut.js). Would probably need to add a new global variable for this that is set in matomo-proxy.php & handled in proxy.php.

@Skywalker-11 commented on May 23rd 2018 Contributor

@mattab Can you take an other look at the branches. I changed the proxy.php to replace the path in the content to point to the matomo-proxy.php?file=<path>.

If that is ok with you I would create a new PR for the proxy and master

@tsteur commented on May 23rd 2018 Member

Was this closed by accident @mattab ?

@diosmosis commented on May 28th 2018 Member

BTW, if you could create a PR for your tracker-proxy changes (which in general look good), that would be helpful.

@Skywalker-11 commented on May 28th 2018 Contributor

Yes files shouldn't be there. Maybe leftovers from setup or phpunit.
PR 42 of matomo-proxy referenced above should contain that changes. Do you want to reopen that PR or shall I create a new one?

@diosmosis commented on May 29th 2018 Member

I'll reopen it.

This Pull Request was closed on May 30th 2018
Powered by GitHub Issue Mirror