MDL-33950 check if source file is accessible in repository_ajax.php
authorMarina Glancy <marina@moodle.com>
Thu, 28 Jun 2012 03:50:17 +0000 (11:50 +0800)
committerMarina Glancy <marina@moodle.com>
Fri, 6 Jul 2012 01:44:07 +0000 (09:44 +0800)
- repository::copy_to_area() does not check access any more, and repository_recent::copy_to_area() is unnecessary
- added repository::file_is_accessible() that checks access to the picked file (regardless of accessibility of the file it is referencing to)

repository/lib.php
repository/recent/lib.php
repository/repository_ajax.php

index 6231209..0886ccb 100644 (file)
@@ -651,45 +651,60 @@ abstract class repository {
      * @return stored_file|null
      */
     public static function get_moodle_file($source) {
-        $params = unserialize(base64_decode($source));
-        if (empty($params) || !is_array($params)) {
-            return null;
-        }
-        foreach (array('contextid', 'itemid', 'filename', 'filepath', 'component') as $key) {
-            if (!array_key_exists($key, $params)) {
-                return null;
+        $params = file_storage::unpack_reference($source, true);
+        $fs = get_file_storage();
+        return $fs->get_file($params['contextid'], $params['component'], $params['filearea'],
+                    $params['itemid'], $params['filepath'], $params['filename']);
+    }
+
+    /**
+     * Repository method to make sure that user can access particular file.
+     *
+     * This is checked when user tries to pick the file from repository to deal with
+     * potential parameter substitutions is request
+     *
+     * @param string $source
+     * @return bool whether the file is accessible by current user
+     */
+    public function file_is_accessible($source) {
+        if ($this->has_moodle_files()) {
+            try {
+                $params = file_storage::unpack_reference($source, true);
+            } catch (file_reference_exception $e) {
+                return false;
             }
+            $browser = get_file_browser();
+            $context = get_context_instance_by_id($params['contextid']);
+            $file_info = $browser->get_file_info($context, $params['component'], $params['filearea'],
+                    $params['itemid'], $params['filepath'], $params['filename']);
+            return !empty($file_info);
         }
-        $contextid  = clean_param($params['contextid'], PARAM_INT);
-        $component  = clean_param($params['component'], PARAM_COMPONENT);
-        $filearea   = clean_param($params['filearea'],  PARAM_AREA);
-        $itemid     = clean_param($params['itemid'],    PARAM_INT);
-        $filepath   = clean_param($params['filepath'],  PARAM_PATH);
-        $filename   = clean_param($params['filename'],  PARAM_FILE);
-        $fs = get_file_storage();
-        return $fs->get_file($contextid, $component, $filearea, $itemid, $filepath, $filename);
+        return true;
     }
 
     /**
-     * This function is used to copy a moodle file to draft area
+     * This function is used to copy a moodle file to draft area.
+     *
+     * It DOES NOT check if the user is allowed to access this file because the actual file
+     * can be located in the area where user does not have access to but there is an alias
+     * to this file in the area where user CAN access it.
+     * {@link file_is_accessible} should be called for alias location before calling this function.
      *
-     * @param string $encoded The metainfo of file, it is base64 encoded php serialized data
+     * @param string $source The metainfo of file, it is base64 encoded php serialized data
      * @param stdClass|array $filerecord contains itemid, filepath, filename and optionally other
      *      attributes of the new file
      * @param int $maxbytes maximum allowed size of file, -1 if unlimited. If size of file exceeds
      *      the limit, the file_exception is thrown.
-     * @return array The information of file
+     * @return array The information about the created file
      */
-    public function copy_to_area($encoded, $filerecord, $maxbytes = -1) {
+    public function copy_to_area($source, $filerecord, $maxbytes = -1) {
         global $USER;
         $fs = get_file_storage();
-        $browser = get_file_browser();
 
         if ($this->has_moodle_files() == false) {
             throw new coding_exception('Only repository used to browse moodle files can use repository::copy_to_area()');
         }
 
-        $params = unserialize(base64_decode($encoded));
         $user_context = context_user::instance($USER->id);
 
         $filerecord = (array)$filerecord;
@@ -701,17 +716,9 @@ abstract class repository {
         $new_filepath = $filerecord['filepath'];
         $new_filename = $filerecord['filename'];
 
-        $contextid  = clean_param($params['contextid'], PARAM_INT);
-        $fileitemid = clean_param($params['itemid'],    PARAM_INT);
-        $filename   = clean_param($params['filename'],  PARAM_FILE);
-        $filepath   = clean_param($params['filepath'],  PARAM_PATH);;
-        $filearea   = clean_param($params['filearea'],  PARAM_AREA);
-        $component  = clean_param($params['component'], PARAM_COMPONENT);
-
-        $context    = get_context_instance_by_id($contextid);
         // the file needs to copied to draft area
-        $file_info  = $browser->get_file_info($context, $component, $filearea, $fileitemid, $filepath, $filename);
-        if ($maxbytes !== -1 && $file_info->get_filesize() > $maxbytes) {
+        $stored_file = self::get_moodle_file($source);
+        if ($maxbytes != -1 && $stored_file->get_filesize() > $maxbytes) {
             throw new file_exception('maxbytes');
         }
 
@@ -719,7 +726,7 @@ abstract class repository {
             // create new file
             $unused_filename = repository::get_unused_filename($draftitemid, $new_filepath, $new_filename);
             $filerecord['filename'] = $unused_filename;
-            $file_info->copy_to_storage($filerecord);
+            $fs->create_file_from_storedfile($filerecord, $stored_file);
             $event = array();
             $event['event'] = 'fileexists';
             $event['newfile'] = new stdClass;
@@ -729,17 +736,17 @@ abstract class repository {
             $event['existingfile'] = new stdClass;
             $event['existingfile']->filepath = $new_filepath;
             $event['existingfile']->filename = $new_filename;
-            $event['existingfile']->url      = moodle_url::make_draftfile_url($draftitemid, $new_filepath, $new_filename)->out();;
+            $event['existingfile']->url = moodle_url::make_draftfile_url($draftitemid, $new_filepath, $new_filename)->out();
             return $event;
         } else {
-            $file_info->copy_to_storage($filerecord);
+            $fs->create_file_from_storedfile($filerecord, $stored_file);
             $info = array();
             $info['itemid'] = $draftitemid;
-            $info['file']  = $new_filename;
-            $info['title']  = $new_filename;
+            $info['file'] = $new_filename;
+            $info['title'] = $new_filename;
             $info['contextid'] = $user_context->id;
-            $info['url'] = moodle_url::make_draftfile_url($draftitemid, $new_filepath, $new_filename)->out();;
-            $info['filesize'] = $file_info->get_filesize();
+            $info['url'] = moodle_url::make_draftfile_url($draftitemid, $new_filepath, $new_filename)->out();
+            $info['filesize'] = $stored_file->get_filesize();
             return $info;
         }
     }
index 2c1acf2..f1265c8 100644 (file)
@@ -177,91 +177,22 @@ class repository_recent extends repository {
     public function supported_returntypes() {
         return FILE_INTERNAL;
     }
+
     /**
-     * This function overwrite the default implement to copying file using file_storage
+     * Repository method to make sure that user can access particular file.
+     *
+     * This is checked when user tries to pick the file from repository to deal with
+     * potential parameter substitutions is request
      *
-     * @param string $encoded The information of file, it is base64 encoded php serialized data
-     * @param stdClass|array $filerecord contains itemid, filepath, filename and optionally other
-     *      attributes of the new file
-     * @param int $maxbytes maximum allowed size of file, -1 if unlimited. If size of file exceeds
-     *      the limit, the file_exception is thrown.
-     * @return array The information of file
+     * @todo MDL-33805 remove this function when recent files are managed correctly
+     *
+     * @param string $source
+     * @return bool whether the file is accessible by current user
      */
-    public function copy_to_area($encoded, $filerecord, $maxbytes = -1) {
+    public function file_is_accessible($source) {
         global $USER;
-
-        $user_context = get_context_instance(CONTEXT_USER, $USER->id);
-
-        $filerecord = (array)$filerecord;
-        // make sure the new file will be created in user draft area
-        $filerecord['component'] = 'user'; // make sure
-        $filerecord['filearea'] = 'draft'; // make sure
-        $filerecord['contextid'] = $user_context->id;
-        $filerecord['sortorder'] = 0;
-        $draftitemid = $filerecord['itemid'];
-        $new_filepath = $filerecord['filepath'];
-        $new_filename = $filerecord['filename'];
-
-        $fs = get_file_storage();
-
-        $params = unserialize(base64_decode($encoded));
-
-        $contextid  = clean_param($params['contextid'], PARAM_INT);
-        $fileitemid = clean_param($params['itemid'],    PARAM_INT);
-        $filename   = clean_param($params['filename'],  PARAM_FILE);
-        $filepath   = clean_param($params['filepath'],  PARAM_PATH);;
-        $filearea   = clean_param($params['filearea'],  PARAM_AREA);
-        $component  = clean_param($params['component'], PARAM_COMPONENT);
-
-        // XXX:
-        // When user try to pick a file from other filearea, normally file api will use file browse to
-        // operate the files with capability check, but in some areas, users don't have permission to
-        // browse the files (for example, forum_attachment area).
-        //
-        // To get 'recent' plugin working, we need to use lower level file_stoarge class to bypass the
-        // capability check, we will use a better workaround to improve it.
-        // TODO MDL-33297 apply here
-        if ($stored_file = $fs->get_file($contextid, $component, $filearea, $fileitemid, $filepath, $filename)) {
-            // verify user id
-            if ($USER->id != $stored_file->get_userid()) {
-                throw new moodle_exception('errornotyourfile', 'repository');
-            }
-            if ($maxbytes !== -1 && $stored_file->get_filesize() > $maxbytes) {
-                throw new file_exception('maxbytes');
-            }
-
-            // test if file already exists
-            if (repository::draftfile_exists($draftitemid, $new_filepath, $new_filename)) {
-                // create new file
-                $unused_filename = repository::get_unused_filename($draftitemid, $new_filepath, $new_filename);
-                $filerecord['filename'] = $unused_filename;
-                // create a tmp file
-                $fs->create_file_from_storedfile($filerecord, $stored_file);
-                $event = array();
-                $event['event'] = 'fileexists';
-                $event['newfile'] = new stdClass;
-                $event['newfile']->filepath = $new_filepath;
-                $event['newfile']->filename = $unused_filename;
-                $event['newfile']->url = moodle_url::make_draftfile_url($draftitemid, $new_filepath, $unused_filename)->out();
-                $event['existingfile'] = new stdClass;
-                $event['existingfile']->filepath = $new_filepath;
-                $event['existingfile']->filename = $new_filename;
-                $event['existingfile']->url      = moodle_url::make_draftfile_url($draftitemid, $new_filepath, $new_filename)->out();;
-                return $event;
-            } else {
-                $fs->create_file_from_storedfile($filerecord, $stored_file);
-                $info = array();
-                $info['title']  = $new_filename;
-                $info['file']  = $new_filename;
-                $info['itemid'] = $draftitemid;
-                $info['filesize']  = $stored_file->get_filesize();
-                $info['url'] = moodle_url::make_draftfile_url($draftitemid, $new_filepath, $new_filename)->out();;
-                $info['contextid'] = $user_context->id;
-                return $info;
-            }
-        }
-        return false;
-
+        $file = self::get_moodle_file($source);
+        return (!empty($file) && $file->get_userid() == $USER->id);
     }
 
     /**
index c9863c4..488ff2f 100644 (file)
@@ -232,7 +232,14 @@ switch ($action) {
             $record->userid = $USER->id;
             $record->sortorder = 0;
 
+            // Check that user has permission to access this file
+            if (!$repo->file_is_accessible($source)) {
+                throw new file_exception('storedfilecannotread');
+            }
+
             // If file is already a reference, set $source = file source, $repo = file repository
+            // note that in this case user may not have permission to access the source file directly
+            // so no file_browser/file_info can be used below
             if ($repo->has_moodle_files()) {
                 $file = repository::get_moodle_file($source);
                 if ($file && $file->is_external_file()) {
@@ -298,13 +305,13 @@ switch ($action) {
             } else {
                 // Download file to moodle.
                 $downloadedfile = $repo->get_file($source, $saveas_filename);
-                if ($downloadedfile['path'] === false) {
+                if (empty($downloadedfile['path'])) {
                     $err->error = get_string('cannotdownload', 'repository');
                     die(json_encode($err));
                 }
 
                 // Check if exceed maxbytes.
-                if (($maxbytes!==-1) && (filesize($downloadedfile['path']) > $maxbytes)) {
+                if ($maxbytes != -1 && filesize($downloadedfile['path']) > $maxbytes) {
                     throw new file_exception('maxbytes');
                 }