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

Ensure that Volunteer Report instance can only be accessed by people … #517

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

seamuslee001
Copy link
Contributor

…with access to CiviReport

At present if someone was to go to the public url civicrm/report/list they would potentially be able to create copies, run the report etc. This ensures that you have to at least access CiviReport to do anything with the report

ping @JKingsnorth @totten @eileenmcnaughton @ginkgomzd

@seamuslee001
Copy link
Contributor Author

also ping @mlutfy

@ginkgomzd
Copy link
Contributor

Thanks @seamuslee001 !
But, are you sure this is needed?
I'm spot checking on one instance I don't see that a user without Access CiviReports can see anything at that URL.
I don't get access denied, but it says, "No reports have been created. Contact your site administrator for help creating reports.", which is not true, so seems fine without this change.

@seamuslee001
Copy link
Contributor Author

@ginkgomzd
Copy link
Contributor

huh, that's interesting... I just tried a bunch of random civicrm sites... and none of them are displaying that. Was there a change in the permission checks in core?

I'm not against the change, just curious what is going on.

@seamuslee001
Copy link
Contributor Author

I'm not sure maybe @mlutfy can check and see what is going on perhaps. I haven't seen any changes to this https://github.com/civicrm/civicrm-core/blob/master/CRM/Report/Page/InstanceList.php#L164 or https://github.com/civicrm/civicrm-core/blob/master/CRM/Report/Utils/Report.php#L346 to suggest that there have been changes in the core code but not sure

@mlutfy
Copy link
Member

mlutfy commented Aug 2, 2019

It's odd: the civicrm_report_instance table had empty values for the permissions/roles. I re-saved the report instance, and then it was fine (i.e. saved in DB).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants