Moodle
  1. Moodle
  2. MDL-31789

Teacher can not access a local system file repository in his course.

    Details

    • Database:
      MySQL
    • Testing Instructions:
      Hide

      Test case 1:

      Clean installation Moodle 2.2.1.
      Configure the repositoyr extension with System File repository. And check permission to create a repository folder into a course.
      Create a simple course with one teacher enrol it.
      Create a repository folder into a course.
      Acces Moodle with user teacher into this course. And try to create a file resource.
      Editing form using filepicker to attach a file. In the filepicker teacher can acces a repository folder course.

      Test case 2:

      1. As admin or teacher, open Filepicker inside any course (from TinyMCE or filemanager) and make sure that repository 'Server files' is in the current course folder by default.
      2. As admin enable legacy files in settings
      3. As admin enable Legacy files repository
      4. In one of the courses enable legacy files in settings (last one!)
      5. Add some files to course legacy files
      5. As teacher/admin make sure you can access legacy files in current course from filepicker

      Show
      Test case 1: Clean installation Moodle 2.2.1. Configure the repositoyr extension with System File repository. And check permission to create a repository folder into a course. Create a simple course with one teacher enrol it. Create a repository folder into a course. Acces Moodle with user teacher into this course. And try to create a file resource. Editing form using filepicker to attach a file. In the filepicker teacher can acces a repository folder course. Test case 2: 1. As admin or teacher, open Filepicker inside any course (from TinyMCE or filemanager) and make sure that repository 'Server files' is in the current course folder by default. 2. As admin enable legacy files in settings 3. As admin enable Legacy files repository 4. In one of the courses enable legacy files in settings (last one!) 5. Add some files to course legacy files 5. As teacher/admin make sure you can access legacy files in current course from filepicker
    • Affected Branches:
      MOODLE_22_STABLE, MOODLE_23_STABLE
    • Fixed Branches:
      MOODLE_22_STABLE
    • Pull Master Branch:
      wip-MDL-31789-master
    • Rank:
      38405

      Description

      In Moodle 2.2.1+ latest stable release a teacher can access into filepicker in a local system file repository in his course.
      Moodle 2.2.1+ (Build: 20120223)

      1. Repository.odt
        657 kB
        Beverley Booker
      1. moodle221repository.png
        92 kB

        Issue Links

          Activity

          Carlos Escobedo Orea created issue -
          Carlos Escobedo Orea made changes -
          Field Original Value New Value
          Attachment moodle221repository.png [ 27091 ]
          Carlos Escobedo Orea made changes -
          Labels partner
          Hide
          Carlos Escobedo Orea added a comment -

          Attachments LoginForumView.jmw is wrong, please remove it.

          Show
          Carlos Escobedo Orea added a comment - Attachments LoginForumView.jmw is wrong, please remove it.
          Helen Foster made changes -
          Attachment LoginForumView.jmx [ 27090 ]
          Hide
          Helen Foster added a comment -

          Attachment removed as requested.

          Show
          Helen Foster added a comment - Attachment removed as requested.
          Hide
          Carlos Escobedo Orea added a comment -

          Thanks a lot Helen.

          Show
          Carlos Escobedo Orea added a comment - Thanks a lot Helen.
          Beverley Booker made changes -
          Attachment Repository.odt [ 27350 ]
          Hide
          Beverley Booker added a comment -

          There is a discussion on this:

          http://moodle.org/mod/forum/discuss.php?d=195768

          Allowing server file access to authenticated users is a security issue, this is not an acceptable workaround.

          This is a major bug!

          Show
          Beverley Booker added a comment - There is a discussion on this: http://moodle.org/mod/forum/discuss.php?d=195768 Allowing server file access to authenticated users is a security issue, this is not an acceptable workaround. This is a major bug!
          Stephen Bourget made changes -
          Link This issue has been marked as being related by MDL-31980 [ MDL-31980 ]
          Dongsheng Cai made changes -
          Link This issue is a regression caused by MDL-30452 [ MDL-30452 ]
          Hide
          Heikki Wilenius added a comment -

          Agreed, this should be a major issue.

          What are the security risks in allowing system file repository for authenticated users? We are thinking whether to enable this in our 2.2 test install.

          Show
          Heikki Wilenius added a comment - Agreed, this should be a major issue. What are the security risks in allowing system file repository for authenticated users? We are thinking whether to enable this in our 2.2 test install.
          Hide
          Juan Leyva added a comment -

          This seems to be related to:

          http://tracker.moodle.org/browse/MDL-30452

          Show
          Juan Leyva added a comment - This seems to be related to: http://tracker.moodle.org/browse/MDL-30452
          Hide
          Juan Leyva added a comment -

          This is what happened:

          • This is stopping current Teachers to use their course levels file system repositories
          • The workaround is:
            • Change the authenticated user's repository/filesystem:view at system level to Allow

          Optional:
          – Change the student's repository/filesystem:view at course level to Not allow
          – Change the teacher's repository/filesystem:view at system level to Not Allow

          The current workaround is just to change permissions and test if it works

          I've talked with Dong (the Repos maintainer) and he is going to perform some tests on this issue, once completed, he'll update this issue

          I'm going to ask to Michael also to raise the priority of this issue

          Show
          Juan Leyva added a comment - This is what happened: New Moodle versions since January 2012 has the system authenticated user permissions for view file repositories disabled http://tracker.moodle.org/browse/MDL-30452 This is stopping current Teachers to use their course levels file system repositories The workaround is: Change the authenticated user's repository/filesystem:view at system level to Allow Optional: – Change the student's repository/filesystem:view at course level to Not allow – Change the teacher's repository/filesystem:view at system level to Not Allow The current workaround is just to change permissions and test if it works I've talked with Dong (the Repos maintainer) and he is going to perform some tests on this issue, once completed, he'll update this issue I'm going to ask to Michael also to raise the priority of this issue
          Martin Dougiamas made changes -
          Link This issue is duplicated by MDL-31980 [ MDL-31980 ]
          Martin Dougiamas made changes -
          Link This issue has been marked as being related by MDL-31980 [ MDL-31980 ]
          Martin Dougiamas made changes -
          Priority Minor [ 4 ] Blocker [ 1 ]
          Martin Dougiamas made changes -
          Assignee Dongsheng Cai [ dongsheng ] moodle.com [ moodle.com ]
          Michael de Raadt made changes -
          Fix Version/s STABLE backlog [ 10463 ]
          Labels partner partner triaged
          Hide
          Dongsheng Cai added a comment - - edited

          OK, I figured out, MDL-30452 triggered this issue, but the real problem is the refactor of accesslib.php: MDL-29602.

          context is an object with protected id, level, depth after refactoring, when formlib generated options for filepicker/editor/filemanager using json_encode(), we got an empty object, then no contextid send to filepicker ajax script, when no context id provided, ajax script will assume this is in system context, two solution for this issue:

          1. implement __toString() in context class to dump json string, doesn't look quite nice in this way
          2. Manually construct context object with public properties in formlib, make sure json_encode works as expected
          Show
          Dongsheng Cai added a comment - - edited OK, I figured out, MDL-30452 triggered this issue, but the real problem is the refactor of accesslib.php: MDL-29602 . context is an object with protected id, level, depth after refactoring, when formlib generated options for filepicker/editor/filemanager using json_encode(), we got an empty object, then no contextid send to filepicker ajax script, when no context id provided, ajax script will assume this is in system context, two solution for this issue: implement __toString() in context class to dump json string, doesn't look quite nice in this way Manually construct context object with public properties in formlib, make sure json_encode works as expected
          Dongsheng Cai made changes -
          Link This issue has a non-specific relationship to MDL-29602 [ MDL-29602 ]
          Marina Glancy made changes -
          Assignee moodle.com [ moodle.com ] Marina Glancy [ marina ]
          Marina Glancy made changes -
          Comment [ ugly solution I can think so far of:
          {code}

          diff --git a/lib/editor/tinymce/lib.php b/lib/editor/tinymce/lib.php
          index 77a2756..062dfc5 100644
          --- a/lib/editor/tinymce/lib.php
          +++ b/lib/editor/tinymce/lib.php
          @@ -77,6 +77,9 @@ class tinymce_texteditor extends texteditor {
                   }
                   $PAGE->requires->js_init_call('M.editor_tinymce.init_editor', array($elementid, $this->get_init_params($elementid, $options)), true);
                   if ($fpoptions) {
          + $fpoptions['image']->context = array('id' => $fpoptions['image']->context->id);
          + $fpoptions['media']->context = array('id' => $fpoptions['media']->context->id);
          + $fpoptions['link']->context = array('id' => $fpoptions['link']->context->id);
                       $PAGE->requires->js_init_call('M.editor_tinymce.init_filepicker', array($elementid, $fpoptions), true);
                   }
               }
          diff --git a/lib/form/filemanager.php b/lib/form/filemanager.php
          index 213ecc6..483674c 100644
          --- a/lib/form/filemanager.php
          +++ b/lib/form/filemanager.php
          @@ -365,6 +365,7 @@ function form_filemanager_render($options) {
               } else {
                   $course_maxbytes = $CFG->maxbytes;
               }
          + $options->context = array('id' => $options->context->id);
           
               if ($options->maxbytes == -1 || empty($options->maxbytes)) {
                   $options->maxbytes = $CFG->maxbytes;
          {code}

          I'll have to come back to this issue after my vacation ]
          Rajesh Taneja made changes -
          Link This issue is duplicated by MDL-30869 [ MDL-30869 ]
          Hide
          Rajesh Taneja added a comment -

          Please include testing instructions covering MDL-30869 (Duplicate bug)

          Show
          Rajesh Taneja added a comment - Please include testing instructions covering MDL-30869 (Duplicate bug)
          Dongsheng Cai made changes -
          Link This issue has been marked as being related by MDL-27156 [ MDL-27156 ]
          Marina Glancy made changes -
          Hide
          Marina Glancy added a comment -

          Petr, can you please peer review it.
          This issue is a regression caused by MDL-29602
          Thanks
          Marina

          Show
          Marina Glancy added a comment - Petr, can you please peer review it. This issue is a regression caused by MDL-29602 Thanks Marina
          Marina Glancy made changes -
          Status Open [ 1 ] Waiting for peer review [ 10012 ]
          Peer reviewer skodak
          Pull 2.2 Diff URL https://github.com/marinaglancy/moodle/compare/MOODLE_22_STABLE...wip-MDL-31789-MOODLE_22_STABLE
          Pull 2.2 Branch wip-MDL-31789-MOODLE_22_STABLE
          Marina Glancy made changes -
          Testing Instructions Clean installation Moodle 2.2.1.
          Configure the repositoyr extension with System File repository. And check permission to create a repository folder into a course.
          Create a simple course with one teacher enrol it.
          Create a repository folder into a course.
          Acces Moodle with user teacher into this course. And try to create a file resource.
          Editing form using filepicker to attach a file. In the filepicker teacher can acces a repository folder course.

          Test case 1:

          Clean installation Moodle 2.2.1.
          Configure the repositoyr extension with System File repository. And check permission to create a repository folder into a course.
          Create a simple course with one teacher enrol it.
          Create a repository folder into a course.
          Acces Moodle with user teacher into this course. And try to create a file resource.
          Editing form using filepicker to attach a file. In the filepicker teacher can acces a repository folder course.

          Test case 2:

          1. As admin or teacher, open Filepicker inside any course (from TinyMCE or filemanager) and make sure that repository 'Server files' is in the current course folder by default.
          2. As admin enable legacy files in settings
          3. As admin enable Legacy files repository
          4. In one of the courses enable legacy files in settings (last one!)
          5. Add some files to course legacy files
          5. As teacher/admin make sure you can access legacy files in current course from filepicker
          Hide
          Petr Škoda added a comment -

          +1

          Show
          Petr Škoda added a comment - +1
          Marina Glancy made changes -
          Status Waiting for peer review [ 10012 ] Waiting for integration review [ 10010 ]
          Hide
          Eloy Lafuente (stronk7) added a comment -

          The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

          TIA and ciao

          Show
          Eloy Lafuente (stronk7) added a comment - The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week. TIA and ciao
          Aparup Banerjee made changes -
          Integrator nebgor
          Sam Hemelryk made changes -
          Currently in integration Yes [ 10041 ]
          Aparup Banerjee made changes -
          Status Waiting for integration review [ 10010 ] Integration review in progress [ 10004 ]
          Hide
          Aparup Banerjee added a comment -

          Thanks for this fix Marina.

          This is up for integration testing on MOODLE_22_STABLE and master.

          Show
          Aparup Banerjee added a comment - Thanks for this fix Marina. This is up for integration testing on MOODLE_22_STABLE and master.
          Aparup Banerjee made changes -
          Status Integration review in progress [ 10004 ] Waiting for testing [ 10005 ]
          Affects Version/s 2.2.2 [ 11552 ]
          Affects Version/s 2.3 [ 10657 ]
          Affects Version/s 2.2.1 [ 11456 ]
          Fix Version/s 2.2.3 [ 12053 ]
          Michael de Raadt made changes -
          Tester ankit_frenz
          Ankit Agarwal made changes -
          Status Waiting for testing [ 10005 ] Testing in progress [ 10011 ]
          Hide
          Ankit Agarwal added a comment -

          Hi,
          I and Apu tried to test it, seems to be working expect a few things:-

          ->"In the file-picker teacher can access a repository folder course." I was able to access the file system repo via the file picker, but it displayed only the files that were uploaded to the folder via the linux file system. All files uploaded via moodle (course>folder res>edit>add) doesn't seem to be present here. I guess that is expected behavior?
          -> We are not sure what exactly am supposed to verify by this "make sure that repository 'Server files' is in the current course folder by default."

          -> If you create a file system repo based on a existing folder on your system. You can see its files if you goto course>folder resources>edit>add>filepicker but when you try to access it from tinymce>insert image> upload image> filepicker , the files donot appear. This seems to be an issue even before the patch.

          Thanks

          Show
          Ankit Agarwal added a comment - Hi, I and Apu tried to test it, seems to be working expect a few things:- ->"In the file-picker teacher can access a repository folder course." I was able to access the file system repo via the file picker, but it displayed only the files that were uploaded to the folder via the linux file system. All files uploaded via moodle (course>folder res>edit>add) doesn't seem to be present here. I guess that is expected behavior? -> We are not sure what exactly am supposed to verify by this "make sure that repository 'Server files' is in the current course folder by default." -> If you create a file system repo based on a existing folder on your system. You can see its files if you goto course>folder resources>edit>add>filepicker but when you try to access it from tinymce>insert image> upload image> filepicker , the files donot appear. This seems to be an issue even before the patch. Thanks
          Hide
          Dan Poltawski added a comment -

          These testing instructions are confusing

          All files uploaded via moodle (course>folder res>edit>add) doesn't seem to be present here. I guess that is expected behavior?

          Yes - the filesystem repository only contains files uploaded to the filesystem, it does not interact with moodle files uploads.

          We are not sure what exactly am supposed to verify by this "make sure that repository 'Server files' is in the current course folder by default."

          I think that this means ensure that the server files repository is enabled for the current course.

          If you create a file system repo based on a existing folder on your system. You can see its files if you goto course>folder resources>edit>add>filepicker but when you try to access it from tinymce>insert image> upload image> filepicker , the files donot appear. This seems to be an issue even before the patch.

          Yes, this repository doesn't define 'supported_filetypes' I do not think this testing instruciton is valid and so I think that this can be passed.

          Show
          Dan Poltawski added a comment - These testing instructions are confusing All files uploaded via moodle (course>folder res>edit>add) doesn't seem to be present here. I guess that is expected behavior? Yes - the filesystem repository only contains files uploaded to the filesystem, it does not interact with moodle files uploads. We are not sure what exactly am supposed to verify by this "make sure that repository 'Server files' is in the current course folder by default." I think that this means ensure that the server files repository is enabled for the current course. If you create a file system repo based on a existing folder on your system. You can see its files if you goto course>folder resources>edit>add>filepicker but when you try to access it from tinymce>insert image> upload image> filepicker , the files donot appear. This seems to be an issue even before the patch. Yes, this repository doesn't define 'supported_filetypes' I do not think this testing instruciton is valid and so I think that this can be passed.
          Hide
          Ankit Agarwal added a comment -

          Had a talk with Dan and Rajesh...Looks like this issue can be passed now.
          Thanks

          Show
          Ankit Agarwal added a comment - Had a talk with Dan and Rajesh...Looks like this issue can be passed now. Thanks
          Ankit Agarwal made changes -
          Status Testing in progress [ 10011 ] Tested [ 10006 ]
          Hide
          Aparup Banerjee added a comment -

          The code here has been spread to upstream moodle repositories and mirrors for anyone to use .

          Closing, have a good weekend!

          Show
          Aparup Banerjee added a comment - The code here has been spread to upstream moodle repositories and mirrors for anyone to use . Closing, have a good weekend!
          Aparup Banerjee made changes -
          Status Tested [ 10006 ] Closed [ 6 ]
          Resolution Fixed [ 1 ]
          Currently in integration Yes [ 10041 ]
          Integration date 05/Apr/12
          Rajesh Taneja made changes -
          Comment [ One of the linked issues (MDL-30869) was reported for 2.1. Can you please suggest why this was not ported on 2.1 ? ]
          Hide
          Gabriel Mazetto added a comment -

          convert_to_array messes with valid data.

          For example, it doesn't accept:

          array('first' => array('second'), 'third' => array('second'));

          Please check MDL-35001 for more details.

          Show
          Gabriel Mazetto added a comment - convert_to_array messes with valid data. For example, it doesn't accept: array('first' => array('second'), 'third' => array('second')); Please check MDL-35001 for more details.
          Gabriel Mazetto made changes -
          Link This issue caused a regression MDL-34518 [ MDL-34518 ]
          Gabriel Mazetto made changes -
          Link This issue testing discovered MDL-35001 [ MDL-35001 ]
          Hide
          Jorge Ramos added a comment -

          Hi! We've upgrade 2.2.7 -> 2.2.8 and this bug is suddenly happening, with teacher role we cant access to Server Files nor Legacy Files...

          Can you reopened the issue??

          Show
          Jorge Ramos added a comment - Hi! We've upgrade 2.2.7 -> 2.2.8 and this bug is suddenly happening, with teacher role we cant access to Server Files nor Legacy Files... Can you reopened the issue??
          Hide
          Heiko Schach added a comment -

          After upgrading from 2.3.4 to 2.3.5 we have received reports from our teachers.
          Adding file from File picker doesn't work as it did before. The user gets an error: "No permission to access this repository."

          Steps to reproduce:
          Log in as teacher
          Turn editing on
          Add a resource... > File
          Content > Select Files > Add...

          Error message: "No permission to access this repository."

          Show
          Heiko Schach added a comment - After upgrading from 2.3.4 to 2.3.5 we have received reports from our teachers. Adding file from File picker doesn't work as it did before. The user gets an error: "No permission to access this repository." Steps to reproduce: Log in as teacher Turn editing on Add a resource... > File Content > Select Files > Add... Error message: "No permission to access this repository."
          Hide
          Dan Marsden added a comment -

          Hi Jorge, We don't re-open issues when a patch has been submitted/integrated into code - you should create a new bug describing the issues you are having and can add a link back to this bug as a reference.

          Show
          Dan Marsden added a comment - Hi Jorge, We don't re-open issues when a patch has been submitted/integrated into code - you should create a new bug describing the issues you are having and can add a link back to this bug as a reference.
          Hide
          Frédéric Massart added a comment -

          Hi, I have raised the issue MDL-38474 to take care of that error.

          Show
          Frédéric Massart added a comment - Hi, I have raised the issue MDL-38474 to take care of that error.
          Hide
          Jorge Ramos added a comment -

          Thank you very much Dan&Fréderic!!

          Show
          Jorge Ramos added a comment - Thank you very much Dan&Fréderic!!

            People

            • Votes:
              19 Vote for this issue
              Watchers:
              22 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: