Tasks drag to re-order without task edit permission
otakikam - July 22, 2009 - 20:07
| Project: | Storm |
| Version: | 6.x-1.23 |
| Component: | Storm Task |
| Category: | bug report |
| Priority: | normal |
| Assigned: | homoludens |
| Status: | needs review |
Description
Not sure if this is a bug, intentional, or an error on my installation.
When a user only has permission to view tasks of their organization they can still drag to re-order the tasks and save the changes.
It's not so much that I don't want my users re-ordering I just think it may add confusion. And this generally seems like an inconsistency in permissions.
Thanks
#1
#2
I agree with otakikam, and will look into this. Users should have as simple interface as possible.
#3
Not regarding the simplicity issue - it is a bit inconsistent with the permissions, and there are several other places in Storm that happens which could do with fixing.
Would it be as simple as an
if (edit permission) { click and drag function } else { standard table }?#4
Should be something like that:
In function theme_stormtask_tasks (in stormtask.theme.inc) only this line is important to make rows draggable:
$row['class'] = empty($row['class']) ? 'draggable' : $row['class'] .' draggable';and in function stormtask_tasks_form (in stormtask.admin.inc) omitting this code will prevent user form saving.
$form['submit'] = array('#type' => 'submit',
'#submit' => array('stormtask_tasks_submit'),
'#value' => t('Save'),
);
but that is only template side, there is more work to make it really secure. This two changes should fix presentation side but I searching how to really prevent (even malicious) users from modifying list.
Would it be enough to add check for permissions in function stormtask_tasks_submit (in stormtask.admin.inc) or it is enough not to show $form['submit'] ?
#5
Personally, I'd do both, i.e. add permission check for display and submit just to make sure.
#6
Just to add an extra question - say a user has the permission 'edit own', how should this be handled for this purpose?
---
I'd like to solve this issue before the release of 6.x-1.24.
(See #540212: Testing of -dev version before 6.x-1.24 release)
#7
The issue that I mentioned previously relates to that if a user does not have a simple edit permission, it may be that the user is supposed to be allowed to be edit some nodes that are listed and not others. Therefore, in the attached patches, I have simply used the user_access('stormtask: edit all') rather than stormtask_user_access().
However - it does seem to be a bit of a hack method, which I don't like all that much (small changes all over the place).
#8
Did anyone have any more thoughts on this one?
#9
As this only applies to a projects task, can we check permissions on the project instead of all tasks, i.e. if they're allowed to edit the project then they're allowed to re-order the tasks?
#10
Sounds like it might be better.
In terms of the node and where the data is stored - we'd be bypassing the permissions by changing the weight and/or parent task fields when the user may not have permission to, but conceptually I guess those fields only have a relevance when seen together on the project.