@mattab opened this Issue on October 10th 2020 Member

the new security feature in https://github.com/matomo-org/matomo/pull/16263 would break use cases for users who need to be able to create/update entities like goals and custom dimensions from within an iframe. This is the case when users embed the whole Matomo App within their products. as quickly described in this faq). Currently they would get the message This user has at least some write access. Only tokens of users who have only view access can be used..

Initially was thinking we could have yet another INI setting, but actually it looks better/easier to accept write/admin tokens whenever enable_framed_pages=1 or enable_framed_settings=1 are set in the config?

(need to be added to RC as this would otherwise be a BC break for some)

@mattab commented on October 10th 2020 Member

At same time, could we change the message from This user has at least some write access. Only tokens of users who have only view access can be used. to This user has write or admin access to the website. When embedding reports, for security reasons only view access (read-only) auth tokens can be used. (or better! maybe could use the "permission" wording used eg. https://matomo.org/faq/general/faq_26910/)

@tsteur commented on October 11th 2020 Member

Initially was thinking we could have yet another INI setting, but actually it looks better/easier to accept write/admin tokens whenever enable_framed_pages=1 or enable_framed_settings=1 are set in the config?

We won't do this because those things aren't related. Also embedding manage goals or so is not officially supported by our widgets that can be embedded.

It's likely a use case we don't want to support. There could be some setting if REALLY needed like [General]allow_risky_widget_authentication_using_higher_privileges=1 or something but I prefer not to start supporting this suddenly. (just an example the name, not sure what to use best)

It wouldn't even be a BC break as we never supported this AFAIK. Maybe what should be used in this case is rather the logme feature or so if the entire UI is embedded.

@mattab commented on October 12th 2020 Member

We did support this use case and several people rely on it. That's why we have the feature enable_framed_pages=1. it lets people embed the whole Matomo app without any restrictions that may exist in Widgetize. A new INI setting sounds good too, as long as there's some option for people who need it.

@tsteur commented on October 12th 2020 Member

If we continue to support this (which I would maybe not recommend) then the risks will need to mentioned clearly eg that the token will appear in logs and everything. It's not really secure in various ways (token appearing in logs, not clear who makes changes / anyone being able to misuse things in many ways, etc) plus it can create a lot of work for us to support this as it means we cannot always rely on a session etc.

Also we need to mention on HackerOne that we exclude anything related this config flag (and enable_framed_pages etc) from the security bounty program

@mattab commented on October 13th 2020 Member

it would be ideal if we could keep this feature as it helps people embed Matomo app within their own backoffice (for example in combination with a custom theme, we've seen some really great looking integrations before). Also some people want to embed Matomo App within iframe, including the login form (the token_auth support is not needed in this case, but they need the iframe - they may also use the logme technique).

So we need to be way more transparent of all the disadvantages (which we weren't quite aware of before) and sounds good to mention clearly the security risks:

  • In the INI global config file - todo
  • in the FAQ at https://matomo.org/faq/troubleshooting/faq_147/ - just now edited it a bit to clarify. anything to add?
  • Also updated https://matomo.org/faq/how-to/faq_92/ to redirect to the other faq
  • mention on HackerOne that we exclude anything related this config flag (and enable_framed_pages etc) - todo
  • btw we should still block "Super Users" tokens from being used even when enable_framed_pages is set (similarly to how logme doesn't allow super users)
@tsteur commented on October 13th 2020 Member

@mattab there may be many more security issues. Eg around sessions etc. It's basically quite unpredictable. I wouldn't even be surprised if a user stayed logged in even if the user itself was removed etc.

Powered by GitHub Issue Mirror