Skip to content
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

Check site permission should check if site actually exists #15264

Merged
merged 1 commit into from Jan 15, 2020

Conversation

tsteur
Copy link
Member

@tsteur tsteur commented Dec 12, 2019

Noticed this error in the logs:

Error: {"message":"Call to a member function getName() on null","file":"/core/Plugin/Controller.php","line":622,"request_id":"57488","backtrace":" on l/core/Plugin/Controller.php(622)\n#0 l/core/Plugin/Controller.php(607): Piwik\Plugin\Controller->setGeneralVariablesViewAs(Object(Piwik\View), 'basic')\n#1

The request was like this:

/index.php?module=Widgetize&action=iframe&moduleToWidgetize=Dashboard&actionToWidgetize=index&idSite=254&period=week&date=today&token_auth=XYZANONYMIZED

So you'd think the checkSitePermission should have failed but it didn't because it was requested by a super user and checkUserHasViewAccess does not actually check if the site exists for a super user.

Noticed this error in the logs:

> Error: {"message":"Call to a member function getName() on null","file":"\/core\/Plugin\/Controller.php","line":622,"request_id":"57488","backtrace":" on l\/core\/Plugin\/Controller.php(622)\n#0 l\/core\/Plugin\/Controller.php(607): Piwik\\Plugin\\Controller->setGeneralVariablesViewAs(Object(Piwik\\View), 'basic')\n#1

The request was like this:

> /index.php?module=Widgetize&action=iframe&moduleToWidgetize=Dashboard&actionToWidgetize=index&idSite=254&period=week&date=today&token_auth=XYZANONYMIZED

So you'd think the `checkSitePermission` should have failed but it didn't because it was requested by a super user and `checkUserHasViewAccess` does not actually check if the site exists for a super user.
@tsteur tsteur added the Bug For errors / faults / flaws / inconsistencies etc. label Dec 12, 2019
@tsteur tsteur added this to the 4.0.0 milestone Dec 12, 2019
@tsteur tsteur changed the base branch from 3.x-dev to 4.x-dev January 14, 2020 03:59
Copy link
Member

@sgiehl sgiehl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

haven't tested much, but makes sense to change that.

@tsteur tsteur merged commit 77b6000 into 4.x-dev Jan 15, 2020
@tsteur tsteur deleted the checksiteexists branch January 15, 2020 18:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug For errors / faults / flaws / inconsistencies etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants