From a635424f88d5c8e053bc75290766a9545d3f4219 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Petr=20S=CC=8Ckoda?= <commits@skodak.org>
Date: Wed, 23 Oct 2013 11:04:59 +0200
Subject: [PATCH] MDL-42110 use parent directory permissions when installing
 add-ons

---
 admin/tool/installaddon/classes/installer.php  | 18 +++++++++++++++---
 admin/tool/installaddon/deploy.php             |  6 +++++-
 .../tool/installaddon/tests/installer_test.php |  4 ++--
 3 files changed, 22 insertions(+), 6 deletions(-)

diff --git a/admin/tool/installaddon/classes/installer.php b/admin/tool/installaddon/classes/installer.php
index 1a1866c7e40..e759c432705 100644
--- a/admin/tool/installaddon/classes/installer.php
+++ b/admin/tool/installaddon/classes/installer.php
@@ -392,8 +392,10 @@ class tool_installaddon_installer {
      *
      * @param string $source full path to the existing directory
      * @param string $target full path to the new location of the directory
+     * @param int $dirpermissions
+     * @param int $filepermissions
      */
-    public function move_directory($source, $target) {
+    public function move_directory($source, $target, $dirpermissions, $filepermissions) {
 
         if (file_exists($target)) {
             throw new tool_installaddon_installer_exception('err_folder_already_exists', array('path' => $target));
@@ -405,7 +407,16 @@ class tool_installaddon_installer {
             throw new tool_installaddon_installer_exception('err_no_such_folder', array('path' => $source));
         }
 
-        make_writable_directory($target);
+        if (!file_exists($target)) {
+            // Do not use make_writable_directory() here - it is intended for dataroot only.
+            mkdir($target, true);
+            @chmod($target, $dirpermissions);
+        }
+
+        if (!is_writable($target)) {
+            closedir($handle);
+            throw new tool_installaddon_installer_exception('err_folder_not_writable', array('path' => $target));
+        }
 
         while ($filename = readdir($handle)) {
             $sourcepath = $source.'/'.$filename;
@@ -416,10 +427,11 @@ class tool_installaddon_installer {
             }
 
             if (is_dir($sourcepath)) {
-                $this->move_directory($sourcepath, $targetpath);
+                $this->move_directory($sourcepath, $targetpath, $dirpermissions, $filepermissions);
 
             } else {
                 rename($sourcepath, $targetpath);
+                @chmod($targetpath, $filepermissions);
             }
         }
 
diff --git a/admin/tool/installaddon/deploy.php b/admin/tool/installaddon/deploy.php
index 7dea2cc415d..b80af832fa7 100644
--- a/admin/tool/installaddon/deploy.php
+++ b/admin/tool/installaddon/deploy.php
@@ -69,6 +69,10 @@ if (file_exists($plugintypepath.'/'.$pluginname)) {
         get_string('invaliddata', 'core_error'));
 }
 
-$installer->move_directory($zipcontentpath.'/'.$pluginname, $plugintypepath.'/'.$pluginname);
+// Copy permissions form the plugin type directory.
+$dirpermissions = fileperms($plugintypepath);
+$filepermissions = ($dirpermissions & 0666); // Strip execute flags.
+
+$installer->move_directory($zipcontentpath.'/'.$pluginname, $plugintypepath.'/'.$pluginname, $dirpermissions, $filepermissions);
 fulldelete($CFG->tempdir.'/tool_installaddon/'.$jobid);
 redirect(new moodle_url('/admin'));
diff --git a/admin/tool/installaddon/tests/installer_test.php b/admin/tool/installaddon/tests/installer_test.php
index 28e37bdd4a8..948294cc92c 100644
--- a/admin/tool/installaddon/tests/installer_test.php
+++ b/admin/tool/installaddon/tests/installer_test.php
@@ -33,7 +33,7 @@ defined('MOODLE_INTERNAL') || die();
  * @copyright 2013 David Mudrak <david@moodle.com>
  * @license   http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
  */
-class tool_installaddon_installer_test extends advanced_testcase {
+class tool_installaddon_installer_testcase extends advanced_testcase {
 
     public function test_get_addons_repository_url() {
         $installer = testable_tool_installaddon_installer::instance();
@@ -137,7 +137,7 @@ class tool_installaddon_installer_test extends advanced_testcase {
         file_put_contents($contentsdir.'/readme.txt', 'Hello world!');
 
         $installer = tool_installaddon_installer::instance();
-        $installer->move_directory($jobroot.'/contents', $jobroot.'/moved');
+        $installer->move_directory($jobroot.'/contents', $jobroot.'/moved', 0777, 0666);
 
         $this->assertFalse(is_dir($jobroot.'/contents'));
         $this->assertTrue(is_file($jobroot.'/moved/sub/folder/readme.txt'));