Commit f5cebdef authored by Hugh Davenport's avatar Hugh Davenport Committed by Son Nguyen

Fix permissions of group area (Bug #1034180)

A user should not be able to view/publish an artefact if
- they don't have view/publish permission of that artefact
- they don't have view permission of all parents of that artefact

A user should not be able to edit an artefact if
- they don't have edit permission of that artefact
- they don't have edit permission of the immediate parent of that artefact
- they don't have view permission of any parents below the immediate

This is similar to the UNIX permissions, you shouldn't be able to view
a directory unless all directories below have read (r) and executeable (x)
bits set. The same for editing, you need write (w) permissions of the
immediate parent, and rx for all parents.

In Mahara, there are no executeable bits, but it can be assumed
that view is basically the same as rw for container artefacts, and the same
as r for non container artefacts.

Change-Id: I4f84aca05dd08d02b05fbe084e4724f78c8681a0
Signed-off-by: default avatarHugh Davenport <hugh@catalyst.net.nz>
parent ed4a69e7
......@@ -67,6 +67,9 @@ function pieform_element_filebrowser(Pieform $form, $element) {
}
$folder = $element['folder'];
if ($group && !pieform_element_filebrowser_view_group_folder($group, $folder)) {
$folder = null;
}
$path = pieform_element_filebrowser_get_path($folder);
$smarty->assign('folder', $folder);
$smarty->assign('foldername', $path[0]->title);
......@@ -1233,6 +1236,24 @@ function pieform_element_filebrowser_edit_group_folder($group, $folder) {
}
function pieform_element_filebrowser_view_group_folder($group, $folder) {
global $USER;
if ($folder) {
if (!$folder instanceof ArtefactTypeFolder) {
$folder = new ArtefactTypeFolder($folder);
}
return $USER->can_view_artefact($folder);
}
require_once(get_config('libroot') . 'group.php');
// Group root directory: use default grouptype artefact permissions
if (!$role = group_user_access($group)) {
return false;
}
$permissions = group_get_default_artefact_permissions($group);
return $permissions[$role]->view;
}
function pieform_element_filebrowser_changeowner(Pieform $form, $element) {
$prefix = $form->get_name() . '_' . $element['name'];
$newtabdata = pieform_element_filebrowser_configure_tabs($element['tabs'], $prefix);
......@@ -1308,6 +1329,12 @@ function pieform_element_filebrowser_changefolder(Pieform $form, $element, $fold
// If changing to a group folder, check whether the user can edit it
if ($g = ($owner ? $group : $form->get_property('group'))) {
if (!pieform_element_filebrowser_view_group_folder($g, $folder)) {
return array(
'error' => true,
'message' => get_string('cannotviewfolder', 'artefact.file'),
);
}
$editgroupfolder = pieform_element_filebrowser_edit_group_folder($g, $folder);
}
......
......@@ -33,6 +33,7 @@ $string['sitefilesloaded'] = 'Site files loaded';
$string['addafile'] = 'Add a file';
$string['archive'] = 'Archive';
$string['bytes'] = 'bytes';
$string['cannotviewfolder'] = 'You do not have permission to view the content of this folder';
$string['cannoteditfolder'] = 'You do not have permission to add content to this folder';
$string['cannoteditfoldersubmitted'] = 'You cannot add content to a folder in a submitted page.';
$string['cannotremovefromsubmittedfolder'] = 'You cannot remove content from a folder in a submitted page.';
......
......@@ -449,10 +449,18 @@ abstract class ArtefactTypeFileBase extends ArtefactType {
$where .= '
AND a.parent = ? ';
$phvals[] = $parentfolderid;
$parent = artefact_instance_from_id($parentfolderid);
$can_view_parent = $USER->can_view_artefact($parent);
if (!$can_view_parent) {
return null;
}
$can_edit_parent = $USER->can_edit_artefact($parent);
}
else {
$where .= '
AND a.parent IS NULL';
$can_edit_parent = true;
$can_view_parent = true;
}
$filedata = get_records_sql_assoc($select . $from . $where . $groupby, $phvals);
......@@ -468,6 +476,18 @@ abstract class ArtefactTypeFileBase extends ArtefactType {
if ($item->size) { // Doing this here now for non-js users
$item->size = ArtefactTypeFile::short_size($item->size, true);
}
if ($group) {
if ($item->author == $USER->get('id')) {
$item->can_edit = 1;
$item->can_view = 1;
$item->can_republish = 1;
}
else {
$item->can_edit = $can_edit_parent && $item->can_edit;
$item->can_view = $can_view_parent && $item->can_view;
$item->can_republish = $can_view_parent && $item->can_republish;
}
}
if ($group && $item->author == $USER->get('id')) {
$item->can_edit = 1; // This will show the delete, edit buttons in filelist, but doesn't change the actual permissions in the checkbox
}
......
......@@ -944,6 +944,12 @@ class User {
}
public function can_view_artefact($a) {
$parent = $a->get_parent_instance();
if ($parent) {
if (!$this->can_view_artefact($parent)) {
return false;
}
}
if ($this->get('admin')
|| ($this->get('id') and $this->get('id') == $a->get('owner'))
|| ($a->get('institution') and $this->is_institutional_admin($a->get('institution')))) {
......@@ -959,7 +965,20 @@ class User {
return false;
}
public function can_edit_artefact($a) {
public function can_edit_artefact($a, $viewparent=false) {
$parent = $a->get_parent_instance();
if ($parent) {
if ($viewparent) {
if (!$this->can_view_artefact($parent)) {
return false;
}
}
else {
if (!$this->can_edit_artefact($parent, true)) {
return false;
}
}
}
if ($this->get('admin')
|| ($this->get('id') and $this->get('id') == $a->get('owner'))
|| ($a->get('institution') and $this->is_institutional_admin($a->get('institution')))) {
......@@ -988,6 +1007,12 @@ class User {
}
public function can_publish_artefact($a) {
$parent = $a->get_parent_instance();
if ($parent) {
if (!$this->can_view_artefact($parent)) {
return false;
}
}
if (($this->get('id') and $this->get('id') == $a->get('owner'))) {
return true;
}
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment