Commit d4f0e071 authored by Nigel McNie's avatar Nigel McNie
Browse files

(#3416) Allow site files to be exported by users too.



This change involves relaxing the check that artefacts can only be
exported if they're owned by the user (naturally, site files are not).
Unfortunately, there's no easy replacement check that I can drop in to
continue to guarantee security. $user->can_view_artefact has two
problems: doesn't respect site files, and is slow.

I added a commented out hypothetical permission checking block and a
comment as to why it wasn't uncommented.
Signed-off-by: default avatarNigel McNie <nigel@catalyst.net.nz>
parent 2229908e
...@@ -202,7 +202,14 @@ abstract class PluginExport extends Plugin { ...@@ -202,7 +202,14 @@ abstract class PluginExport extends Plugin {
// Get the list of artefacts to export // Get the list of artefacts to export
if ($artefacts == self::EXPORT_ALL_ARTEFACTS) { if ($artefacts == self::EXPORT_ALL_ARTEFACTS) {
$tmpartefacts = get_column_sql('SELECT id FROM {artefact} WHERE owner = ? ORDER BY id', array($userid)); $tmpartefacts = get_column_sql('SELECT id
FROM {artefact}
WHERE owner = ?
UNION
SELECT artefact
FROM {view_artefact}
WHERE "view" IN (SELECT id FROM {view} WHERE owner = ?)
ORDER BY id', array($userid, $userid));
$this->artefactexportmode = $artefacts; $this->artefactexportmode = $artefacts;
} }
else { else {
...@@ -241,9 +248,16 @@ abstract class PluginExport extends Plugin { ...@@ -241,9 +248,16 @@ abstract class PluginExport extends Plugin {
if (is_null($artefact)) { if (is_null($artefact)) {
throw new ParamOutOfRangeException("Invalid artefact $a"); throw new ParamOutOfRangeException("Invalid artefact $a");
} }
if ($artefact->get('owner') != $userid) { // This check won't work, at the _least_ because at the time of
throw new UserException("User $userid does not own artefact " . $artefact->get('id')); // writing, can_view_artefact does not support normal users viewing
} // site files. This check is also pretty damn slow. So think twice
// before uncommenting it. I presume if you _are_ uncommenting it,
// it's because you're trying to isloate a security vulnerability
// where a user can export another user's files or something. In
// which case you'll be being careful anyway, I hope.
//if (!$this->user->can_view_artefact($artefact)) {
// throw new SystemException("User $userid does not own artefact " . $artefact->get('id'));
//}
$this->artefacts[$artefact->get('id')] = $artefact; $this->artefacts[$artefact->get('id')] = $artefact;
} }
......
Supports Markdown
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