Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
Now, if you now the url, anyone how is logged in, can view the reports.
Should it be easy to implement report access based on Drupal roles so for example a student who is logged in the system can't view the reports. So when you create a report, you can set the permission who can view the report and which person is not allowed to access it. (It's like setting grants on nodes).
Comment | File | Size | Author |
---|---|---|---|
#10 | forena_access_control.patch | 7.24 KB | metzlerd |
Comments
Comment #1
hgkris CreditAttribution: hgkris commentedI've been looking in the code and maybe the best place should be a control function when loading the report.
This should be done on every report.
Can this be done in the file forena.common.inc when calling reports/% and report_doc/% ?
Comment #2
metzlerd CreditAttribution: metzlerd commentedForena data blocks are designed to be written based on drupal rights not roles. This is as it should be as it meets with the drupal philosophy for rights. The reports are designed to report empty data.
I will not deviate on the primary security model, but am willing to consider optional adjustments. More information on this can be found at:
#1595368: Alert Message for Permission
The list of reports that show up on the reports list is currently determined based on whether you have access to the data blocks contained in the reports. Note that reports by default only show the information that you have access to.
Comment #3
hgkris CreditAttribution: hgkris commentedThanks for the info.
Maybe you can add an 'grant' option on each report, like the module Nodeacces https://drupal.org/project/nodeaccess does with nodes.
This way, it should be easier the grant access to each report if each report behave like a node.
Should this be an option? Or is there an easier way to do that?
Comment #4
hgkris CreditAttribution: hgkris commentedFor now I added
'access research reports' => array('title' => t('Access Research Reports'))
to the function forena_permission() in forena.module so I can restrict access to the specific users.
It can no block specific user groups to load blocks. For example I added
--ACCESS=access research reports
to the sql file so this usergroup can't access this.
Is should be nicer that the whole report wouldn't load in $items['reports/%'] and $items['report_doc/%'] then giving an blank page with just the title or in the last one (report_doc) giving an error.
Maybe when no permission is set to access all datablock on the report, then it's shows a messages like you said in https://drupal.org/node/1595368
Fatal error: Call to a member function xpath() on a non-object in .../sites/all/modules/forena/docformats/FrxXLSDoc.inc on line 38
Comment #5
hgkris CreditAttribution: hgkris commentedShould this not be set by default
'access arguments' => array('access reports'),
in
and
'access reports' => array('title' => t('Access Reports')),
in
in forena.module?
So only people who are allowed to view reports, can view this.
Now it's set access content, but anyone will view the page with or without the data blocks that are accessible.
This is just a security option for reports that have sensitive information.
Comment #6
hgkris CreditAttribution: hgkris commentedFurther I added
to
So you can specify which data source will be accessible for which user role.
You only need to add
--ACCESS=access myrepository data where myrepository is added in the Forena Reports Configuration (url like ../admin/config/content/forena/data).
Is this an option?
This way you can restrict the report to authorised persons only (see post above) and in this post, you can restrict data source, so you can specify which user role can view which data source.
I only need to find a solution for showing an error message (https://drupal.org/node/1595368) when th user doesnt have acces to view all data blocks.
Comment #7
metzlerd CreditAttribution: metzlerd commentedWith regard to #5. It is not true that people will be able to access sensitive data with the access content privileges. No data blocks will return data if they do not have the right specified in the access= comment in each data block. I'm not sure of the need for the 'access content' vs. 'access report' if you have the repository specific rights in place. It's kind of like implementing separate access to the "node" page, which just lists access to nodes. What do we get for adding this complexity of configuration?
This is the general direction I wanted to go (especially the repository specific access), but we need to change the repository manager class to check this IN ADDITION TO the data block provided security. Would you mind changing the permissions to read "access myrepository data" I think the source is redundant.
Dealing with #1595368: Alert Message for Permission is a separate, but related issue. We might want to move that discussion to this issue. I'd be willing in 7.x-4.x to change the default behavior to report an access denied error message if the user doesn't have access to ANY data blocks in the report, which might work for most cases. Agreed?
If you want your code to be included in forena as is, you need to learn to submit a patch for this. That patch should be created against the 7.x-4.x branch as changing the way permissions work is too significant a change to entertain in the 7.x-3.x branch. See https://drupal.org/node/707484 for more info.
Thanks for being willing to contribute.
Dave
Comment #8
hgkris CreditAttribution: hgkris commentedThanks for the info.
Regard to #6. Maybe in #5, this may be to complex, but it added some security in accessing the node. This is maybe indeed security overkill.
Like you asked, I've changed the permission to "access myrepository data" in the post.
About #1595368: Alert Message for Permission, I totally agree. So if the user doesn't have accees to any data blocks in the report, there will appear a alert message.
Within a few weeks, I'm learning to work with git and applying patches.
I'll try to contribute, as soon as possible.
Comment #9
hgkris CreditAttribution: hgkris commentedComment #10
metzlerd CreditAttribution: metzlerd commentedHere is a patch that changes security to be based on repository. In this patch, the code is changed to check for access myrepos data security. It also makes the --ACCESS parameter optional. When not specified, only the new access myrepos data right is required. I have not changed the access denied page, but will work on that in a separate patch.
Comment #11
metzlerd CreditAttribution: metzlerd commentedComment #12
hgkris CreditAttribution: hgkris commentedOK, thanks!