@Valair opened this Issue on May 3rd 2019

This is related to this issue https://github.com/matomo-org/matomo/issues/8697, but with wider implications.

Steps to reproduce: the same as the previous issue, but it works even with plugins (I'm currently working on extending the DisableTracking for example.
Results: if a call to API with not existing idSites is made using super-admin auth token, there is no error message for not existing IDs, as the permission check successfully complete, and when a website ID with that ID is created, all the options are applied.
Expected result: it should not be possible, even for super-admin to use not existing IDs and an exception should be raised.

I think to have tracked the cause of this behaviour on how the access check is made in Piwik::Access class; if the user is a super-admin, all Piwik::Access.checkUserHas*Access($idSites) return as $this->hasSuperUserAccess(), without checking the given IDs.

@tsteur commented on May 5th 2019 Member

@Valair I'll mark this as a duplicate of #8697 as it's pretty much the same issue / root cause. Indeed removing eg changing

    public function checkUserHasViewAccess($idSites)
    {
        if ($this->hasSuperUserAccess()) {
            return;
        }
  . ..

to

    public function checkUserHasViewAccess($idSites)
    {
  ...

should work. The only possible big problem that may happen is that quite a few automated tests may fail (integration tests, system tests) and we would need to adjust them to ensure the sites exist. wouldn't be surprised if some tests only worked due to this bug. We would just remove those lines with if ($this->hasSuperUserAccess()) { and see if many tests fail. Would you be keen to create a PR?

This Issue was closed on May 5th 2019
Powered by GitHub Issue Mirror