Commit f1db0886 authored by Robert Lyon's avatar Robert Lyon Committed by Gerrit Code Review
Browse files

Bug 1211758 - Make sure user has permission to publish artefacts



Change-Id: I262b2460195c663fd461964b9bd5a60efd8b297a
Signed-off-by: Robert Lyon's avatarRobert Lyon <robertl@catalyst.net.nz>
parent b5a36d39
......@@ -309,6 +309,13 @@ function editpost_submit(Pieform $form, $values) {
$old = $postobj->attachment_id_list();
// $new = is_array($values['filebrowser']['selected']) ? $values['filebrowser']['selected'] : array();
$new = is_array($values['filebrowser']) ? $values['filebrowser'] : array();
// only allow the attaching of files that exist and are editable by user
foreach ($new as $key => $fileid) {
$file = artefact_instance_from_id($fileid);
if (!($file instanceof ArtefactTypeFile) || !$USER->can_publish_artefact($file)) {
unset($new[$key]);
}
}
if (!empty($new) || !empty($old)) {
foreach ($old as $o) {
if (!in_array($o, $new)) {
......
......@@ -579,6 +579,21 @@ class PluginBlocktypeGallery extends PluginBlocktype {
);
}
public static function instance_config_validate($form, $values) {
global $USER;
if (!empty($values['images'])) {
foreach ($values['images'] as $id) {
$image = new ArtefactTypeImage($id);
if (!($image instanceof ArtefactTypeImage) || !$USER->can_publish_artefact($image)) {
$result['message'] = get_string('unrecoverableerror', 'error');
$form->set_error(null, $result['message']);
$form->reply(PIEFORM_ERR, $result);
}
}
}
}
public static function instance_config_save($values) {
if ($values['select'] == 0) {
$values['artefactid'] = $values['folder'];
......
......@@ -477,7 +477,13 @@ abstract class ArtefactTypeFileBase extends ArtefactType {
$item->size = ArtefactTypeFile::short_size($item->size, true);
}
if ($group) {
if ($item->author == $USER->get('id')) {
// site public files
if ($institution == 'mahara' && ArtefactTypeFolder::admin_public_folder_id() == $parentfolderid) {
$item->can_edit = 0;
$item->can_view = 1;
$item->can_republish = 1;
}
else if (!empty($item->author) && $item->author == $USER->get('id')) {
$item->can_edit = 1;
$item->can_view = 1;
$item->can_republish = 1;
......@@ -488,8 +494,10 @@ abstract class ArtefactTypeFileBase extends ArtefactType {
$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
if (!empty($item->author)) {
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
}
}
}
$where = 'artefact IN (' . join(',', array_keys($filedata)) . ')';
......
......@@ -473,6 +473,13 @@ EOF;
// Add attachments, if there are any...
$old = $artefact->attachment_id_list();
$new = is_array($values['artefactids']) ? $values['artefactids'] : array();
// only allow the attaching of files that exist and are editable by user
foreach ($new as $key => $fileid) {
$file = artefact_instance_from_id($fileid);
if (!($file instanceof ArtefactTypeFile) || !$USER->can_publish_artefact($file)) {
unset($new[$key]);
}
}
if (!empty($new) || !empty($old)) {
foreach ($old as $o) {
if (!in_array($o, $new)) {
......
......@@ -215,7 +215,7 @@ $smarty->display('artefact:resume:editgoalsandskills.tpl');
function editgoalsandskills_submit(Pieform $form, array $values) {
global $SESSION, $artefact;
global $SESSION, $artefact, $USER;
db_begin();
$artefact->set('title', get_string($values['artefacttype'], 'artefact.resume'));
......@@ -225,6 +225,13 @@ function editgoalsandskills_submit(Pieform $form, array $values) {
// Attachments
$old = $artefact->attachment_id_list();
$new = is_array($values['filebrowser']) ? $values['filebrowser'] : array();
// only allow the attaching of files that exist and are editable by user
foreach ($new as $key => $fileid) {
$file = artefact_instance_from_id($fileid);
if (!($file instanceof ArtefactTypeFile) || !$USER->can_publish_artefact($file)) {
unset($new[$key]);
}
}
if (!empty($new) || !empty($old)) {
foreach ($old as $o) {
if (!in_array($o, $new)) {
......
......@@ -595,6 +595,7 @@ abstract class ArtefactTypeResumeComposite extends ArtefactTypeResume {
* TODO: expand on these docs.
*/
public static function ensure_composite_value($values, $compositetype, $owner) {
global $USER;
if (!in_array($compositetype, self::get_composite_artefact_types())) {
throw new SystemException("ensure_composite_value called with invalid composite type");
}
......@@ -685,6 +686,13 @@ abstract class ArtefactTypeResumeComposite extends ArtefactTypeResume {
if (array_key_exists('filebrowser', $values)) {
$old = $a->attachment_id_list_with_item($itemid);
$new = is_array($values['filebrowser']) ? $values['filebrowser'] : array();
// only allow the attaching of files that exist and are editable by user
foreach ($new as $key => $fileid) {
$file = artefact_instance_from_id($fileid);
if (!($file instanceof ArtefactTypeFile) || !$USER->can_publish_artefact($file)) {
unset($new[$key]);
}
}
if (!empty($new) || !empty($old)) {
foreach ($old as $o) {
if (!in_array($o, $new)) {
......
......@@ -945,6 +945,8 @@ class User {
}
public function can_view_artefact($a) {
global $USER;
$parent = $a->get_parent_instance();
if ($parent) {
if (!$this->can_view_artefact($parent)) {
......@@ -956,6 +958,15 @@ class User {
|| ($a->get('institution') and $this->is_institutional_admin($a->get('institution')))) {
return true;
}
// public site files
else if ($a->get('institution') == 'mahara') {
$thisparent = $a->get('parent');
// if we are looking at the public folder or items in it
if (($a->get('id') == ArtefactTypeFolder::admin_public_folder_id())
|| (!empty($thisparent) && $thisparent == ArtefactTypeFolder::admin_public_folder_id())) {
return true;
}
}
if ($a->get('group')) {
// Only group artefacts can have artefact_access_role & artefact_access_usr records
return (bool) count_records_sql("SELECT COUNT(*) FROM {artefact_access_role} ar
......
......@@ -514,8 +514,31 @@ class BlockInstance {
throw new ParamOutOfRangeException("Field $field wasn't found in class " . get_class($this));
}
// returns false if it finds a bad attachment
// returns true if all attachments are allowed
private function verify_attachment_permissions($id) {
global $USER;
if (is_array($id)) {
foreach ($id as $id) {
$file = artefact_instance_from_id($id);
if (!$USER->can_publish_artefact($file)) {
// bail out now as at least one attachment is bad
return false;
}
}
}
else {
$file = artefact_instance_from_id($id);
if (!$USER->can_publish_artefact($file)) {
return false;
}
}
return true;
}
public function instance_config_store(Pieform $form, $values) {
global $SESSION;
global $SESSION, $USER;
// Destroy form values we don't care about
unset($values['sesskey']);
......@@ -526,6 +549,22 @@ class BlockInstance {
unset($values['change']);
unset($values['new']);
// make sure that user is allowed to publish artefact. This is to stop
// hacking of form value to attach other users private data.
$badattachment = false;
if (!empty($values['artefactid'])) {
$badattachment = !$this->verify_attachment_permissions($values['artefactid']);
}
if (!empty($values['artefactids'])) {
$badattachment = !$this->verify_attachment_permissions($values['artefactids']);
}
if ($badattachment) {
$result['message'] = get_string('unrecoverableerror', 'error');
$form->set_error(null, $result['message']);
$form->reply(PIEFORM_ERR, $result);
exit();
}
$redirect = '/view/blocks.php?id=' . $this->get('view');
if (param_boolean('new', false)) {
$redirect .= '&new=1';
......
......@@ -100,6 +100,16 @@ function pieform_element_filebrowser(Pieform $form, $element) {
else {
$value = null;
}
// check to see if attached artefact items in $value array are actually allowed
// to be seen by this user
if (!empty($value)) {
foreach ($value as $k => $v) {
$file = artefact_instance_from_id($v);
if (!($file instanceof ArtefactTypeFile) || !$USER->can_publish_artefact($file)) {
unset($value[$k]);
}
}
}
$selected = $element['selectlistcallback']($value);
}
$smarty->assign('selectedlist', $selected);
......
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