Moodle
  1. Moodle
  2. MDL-36963

Automatic updates deployer needs to check directory permisisons too

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Critical Critical
    • Resolution: Fixed
    • Affects Version/s: 2.4
    • Fix Version/s: 2.4.1
    • Component/s: Administration
    • Labels:
    • Testing Instructions:
      Hide

      Testing difficulty: HARD (requires shell access to the server and directory permission modifications)

      1. Prepare a 2.4 site with auto updates deployment enabled.
      2. Install a non-recent version of an add-on from the Plugins directory (so that it can be updated automatically)
      3. Set the permissions at the server so that: (a) the web server process has write access to the plugin folder and all its content (e.g. /mod/foobar) but (b) the web server process does not have write access to the parent folder (e.g. /mod in this case).
      4. TEST: Make sure that the plugin is updatable via the admin interface
      Show
      Testing difficulty: HARD (requires shell access to the server and directory permission modifications) Prepare a 2.4 site with auto updates deployment enabled. Install a non-recent version of an add-on from the Plugins directory (so that it can be updated automatically) Set the permissions at the server so that: (a) the web server process has write access to the plugin folder and all its content (e.g. /mod/foobar) but (b) the web server process does not have write access to the parent folder (e.g. /mod in this case). TEST: Make sure that the plugin is updatable via the admin interface
    • Affected Branches:
      MOODLE_24_STABLE
    • Fixed Branches:
      MOODLE_24_STABLE
    • Pull from Repository:
    • Pull 2.4 Branch:
      MDL-36963-deployment-backup_24
    • Rank:
      46504

      Description

      While testing MDL-36903, I ended up with the following exception:

      Moodle deployment utility had a trouble with your request. See the docs page and the debugging information for more details.
      
      exception 'backup_folder_exception' with message 'Unable to backup the current version of the plugin (moving failed)' in mdeploy.php:755
      Stack trace:
      #0 mdeploy.php(1326): worker->execute()
      #1 {main}
      

      This seemed to be caused by:
      1/ web-server write permissions on the plugin directory
      2/ non-write permissions on the containing directory of the plugin

      I eneded with up with:

      • An empty course/format/topcol directory
      • An archived course/format/topcol directory

        Issue Links

          Activity

          Hide
          David Mudrak added a comment -

          Good catch Dan!

          Instead of changing the pre-check and requiring the write access to the parent folder of the plugin, I have modified the mdeploy.php utility so that is does not attempt to move/remove the root folder of a plugin. It just operates with its contents now. So it is enough to give the web server process write access to the plugin folder itself now (as was always supposed to be).

          DEAR INTEGRATORS,

          please apply the patchset to the MOODLE_24_STABLE and master.

          Show
          David Mudrak added a comment - Good catch Dan! Instead of changing the pre-check and requiring the write access to the parent folder of the plugin, I have modified the mdeploy.php utility so that is does not attempt to move/remove the root folder of a plugin. It just operates with its contents now. So it is enough to give the web server process write access to the plugin folder itself now (as was always supposed to be). DEAR INTEGRATORS, please apply the patchset to the MOODLE_24_STABLE and master.
          Hide
          Dan Poltawski added a comment -

          Thanks David, i've integrated (and tested this) now.

          Show
          Dan Poltawski added a comment - Thanks David, i've integrated (and tested this) now.
          Hide
          Dan Poltawski added a comment -

          Leaving this in testing while I try out some other combinations.

          Show
          Dan Poltawski added a comment - Leaving this in testing while I try out some other combinations.
          Hide
          Dan Poltawski added a comment -

          One thing I noticed, if you are coming from a main version number upgrade (i.e. through the pages 'do you want to upgrade to x', 'environment check', 'plugins check', after you install a plugin update on that page you are returned to the go through the steps again. Not sure that is desirable.

          Show
          Dan Poltawski added a comment - One thing I noticed, if you are coming from a main version number upgrade (i.e. through the pages 'do you want to upgrade to x', 'environment check', 'plugins check', after you install a plugin update on that page you are returned to the go through the steps again. Not sure that is desirable.
          Hide
          David Mudrak added a comment -

          Yeah, that could be improved a bit. The mdeploy utility accepts an URL to return to, which should be the plugins check page in this case.

          Show
          David Mudrak added a comment - Yeah, that could be improved a bit. The mdeploy utility accepts an URL to return to, which should be the plugins check page in this case.
          Hide
          David Mudrak added a comment -

          Adding docs_required as this should be noted at http://docs.moodle.org/dev/Moodle_2.4.1_release_notes and the ability to control plugins updatability via permissions should go to http://docs.moodle.org/24/en/Automatic_updates_deployment#Disabling_updates_deployment

          Show
          David Mudrak added a comment - Adding docs_required as this should be noted at http://docs.moodle.org/dev/Moodle_2.4.1_release_notes and the ability to control plugins updatability via permissions should go to http://docs.moodle.org/24/en/Automatic_updates_deployment#Disabling_updates_deployment
          Hide
          Eloy Lafuente (stronk7) added a comment -

          This has landed upstream, closing, thanks!

          Show
          Eloy Lafuente (stronk7) added a comment - This has landed upstream, closing, thanks!
          Hide
          Mary Cooch added a comment -

          (Housekeeping) Removing docs_required label

          Show
          Mary Cooch added a comment - (Housekeeping) Removing docs_required label

            People

            • Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: