install: check file/directory permissions for Shaarli resources

Relates to #40
Relates to #372

Additions:
 - FileUtils: IOException
 - ApplicationUtils:
   - check if Shaarli resources are accessible with sufficient permissions
   - basic test coverage
 - index.php:
   - check access permissions and redirect to an error page if needed:
     - before running the first installation

Modifications:
 - LinkDB:
   - factorize datastore write code
   - check if the datastore
     (exists AND is writeable) OR (doesn't exist AND its parent dir is writable)
   - raise an IOException if needed

Signed-off-by: VirtualTam <virtualtam@flibidi.net>

diff --git a/application/ApplicationUtils.php b/application/ApplicationUtils.php
new file mode 100644 (file)
index 0000000..6fb07f3
--- /dev/null
+++ b/application/ApplicationUtils.php
@@ -0,0 +1,69 @@
+<?php
+/**
+ * Shaarli (application) utilities
+ */
+class ApplicationUtils
+{
+
+    /**
+     * Checks Shaarli has the proper access permissions to its resources
+     *
+     * @param array $globalConfig The $GLOBALS['config'] array
+     *
+     * @return array A list of the detected configuration issues
+     */
+    public static function checkResourcePermissions($globalConfig)
+    {
+        $errors = array();
+
+        // Check script and template directories are readable
+        foreach (array(
+            'application',
+            'inc',
+            'plugins',
+            $globalConfig['RAINTPL_TPL']
+        ) as $path) {
+            if (! is_readable(realpath($path))) {
+                $errors[] = '"'.$path.'" directory is not readable';
+            }
+        }
+
+        // Check cache and data directories are readable and writeable
+        foreach (array(
+            $globalConfig['CACHEDIR'],
+            $globalConfig['DATADIR'],
+            $globalConfig['PAGECACHE'],
+            $globalConfig['RAINTPL_TMP']
+        ) as $path) {
+            if (! is_readable(realpath($path))) {
+                $errors[] = '"'.$path.'" directory is not readable';
+            }
+            if (! is_writable(realpath($path))) {
+                $errors[] = '"'.$path.'" directory is not writable';
+            }
+        }
+
+        // Check configuration files are readable and writeable
+        foreach (array(
+            $globalConfig['CONFIG_FILE'],
+            $globalConfig['DATASTORE'],
+            $globalConfig['IPBANS_FILENAME'],
+            $globalConfig['LOG_FILE'],
+            $globalConfig['UPDATECHECK_FILENAME']
+        ) as $path) {
+            if (! is_file(realpath($path))) {
+                # the file may not exist yet
+                continue;
+            }
+
+            if (! is_readable(realpath($path))) {
+                $errors[] = '"'.$path.'" file is not readable';
+            }
+            if (! is_writable(realpath($path))) {
+                $errors[] = '"'.$path.'" file is not writable';
+            }
+        }
+
+        return $errors;
+    }
+}

diff --git a/application/FileUtils.php b/application/FileUtils.php
new file mode 100644 (file)
index 0000000..6a12ef0
--- /dev/null
+++ b/application/FileUtils.php
@@ -0,0 +1,19 @@
+<?php
+/**
+ * Exception class thrown when a filesystem access failure happens
+ */
+class IOException extends Exception
+{
+    private $path;
+
+    /**
+     * Construct a new IOException
+     *
+     * @param string $path path to the ressource that cannot be accessed
+     */
+    public function __construct($path)
+    {
+        $this->path = $path;
+        $this->message = 'Error accessing '.$this->path;
+    }
+}

diff --git a/application/LinkDB.php b/application/LinkDB.php
index 8473350555e3b8abfdba1a4bd995c9d1da249613..15fadbc3a6baa67e552517b429635aa5f0c79dd1 100644 (file)
--- a/application/LinkDB.php
+++ b/application/LinkDB.php
@@ -212,11 +212,7 @@ You use the community supported version of the original Shaarli project, by Seba
         $this->_links[$link['linkdate']] = $link;
 
         // Write database to disk
-        // TODO: raise an exception if the file is not write-able
-        file_put_contents(
-            $this->_datastore,
-            self::$phpPrefix.base64_encode(gzdeflate(serialize($this->_links))).self::$phpSuffix
-        );
+        $this->writeDB();
     }
 
     /**
@@ -267,6 +263,28 @@ You use the community supported version of the original Shaarli project, by Seba
         }
     }
 
+    /**
+     * Saves the database from memory to disk
+     *
+     * @throws IOException the datastore is not writable
+     */
+    private function writeDB()
+    {
+        if (is_file($this->_datastore) && !is_writeable($this->_datastore)) {
+            // The datastore exists but is not writeable
+            throw new IOException($this->_datastore);
+        } else if (!is_file($this->_datastore) && !is_writeable(dirname($this->_datastore))) {
+            // The datastore does not exist and its parent directory is not writeable
+            throw new IOException(dirname($this->_datastore));
+        }
+
+        file_put_contents(
+            $this->_datastore,
+            self::$phpPrefix.base64_encode(gzdeflate(serialize($this->_links))).self::$phpSuffix
+        );
+
+    }
+
     /**
      * Saves the database from memory to disk
      *
@@ -278,10 +296,9 @@ You use the community supported version of the original Shaarli project, by Seba
             // TODO: raise an Exception instead
             die('You are not authorized to change the database.');
         }
-        file_put_contents(
-            $this->_datastore,
-            self::$phpPrefix.base64_encode(gzdeflate(serialize($this->_links))).self::$phpSuffix
-        );
+
+        $this->writeDB();
+
         invalidateCaches($pageCacheDir);
     }
 

diff --git a/index.php b/index.php
index be181a2c8006208b0b35d9d6a7156da3aa6b4868..654f7f8f36fabb83b84f2e0016b67a8d82e8866b 100644 (file)
--- a/index.php
+++ b/index.php
@@ -44,6 +44,9 @@ $GLOBALS['config']['DATASTORE'] = $GLOBALS['config']['DATADIR'].'/datastore.php'
 // Banned IPs
 $GLOBALS['config']['IPBANS_FILENAME'] = $GLOBALS['config']['DATADIR'].'/ipbans.php';
 
+// Access log
+$GLOBALS['config']['LOG_FILE'] = $GLOBALS['config']['DATADIR'].'/log.txt';
+
 // For updates check of Shaarli
 $GLOBALS['config']['UPDATECHECK_FILENAME'] = $GLOBALS['config']['DATADIR'].'/lastupdatecheck.txt';
 
@@ -52,7 +55,7 @@ $GLOBALS['config']['RAINTPL_TMP'] = 'tmp/';
 // Raintpl template directory (keep the trailing slash!)
 $GLOBALS['config']['RAINTPL_TPL'] = 'tpl/';
 
-// Thuumbnail cache directory
+// Thumbnail cache directory
 $GLOBALS['config']['CACHEDIR'] = 'cache';
 
 // Atom & RSS feed cache directory
@@ -141,8 +144,10 @@ if (is_file($GLOBALS['config']['CONFIG_FILE'])) {
 }
 
 // Shaarli library
+require_once 'application/ApplicationUtils.php';
 require_once 'application/Cache.php';
 require_once 'application/CachedPage.php';
+require_once 'application/FileUtils.php';
 require_once 'application/HttpUtils.php';
 require_once 'application/LinkDB.php';
 require_once 'application/TimeZone.php';
@@ -155,9 +160,9 @@ require_once 'application/Router.php';
 // Ensure the PHP version is supported
 try {
     checkPHPVersion('5.3', PHP_VERSION);
-} catch(Exception $e) {
+} catch(Exception $exc) {
     header('Content-Type: text/plain; charset=utf-8');
-    echo $e->getMessage();
+    echo $exc->getMessage();
     exit;
 }
 
@@ -216,9 +221,6 @@ header("Cache-Control: no-store, no-cache, must-revalidate");
 header("Cache-Control: post-check=0, pre-check=0", false);
 header("Pragma: no-cache");
 
-// Directories creations (Note that your web host may require different rights than 705.)
-if (!is_writable(realpath(dirname(__FILE__)))) die('<pre>ERROR: Shaarli does not have the right to write in its own directory.</pre>');
-
 // Handling of old config file which do not have the new parameters.
 if (empty($GLOBALS['title'])) $GLOBALS['title']='Shared links on '.escape(index_url($_SERVER));
 if (empty($GLOBALS['timezone'])) $GLOBALS['timezone']=date_default_timezone_get();
@@ -228,8 +230,24 @@ if (empty($GLOBALS['privateLinkByDefault'])) $GLOBALS['privateLinkByDefault']=fa
 if (empty($GLOBALS['titleLink'])) $GLOBALS['titleLink']='?';
 // I really need to rewrite Shaarli with a proper configuation manager.
 
-// Run config screen if first run:
 if (! is_file($GLOBALS['config']['CONFIG_FILE'])) {
+    // Ensure Shaarli has proper access to its resources
+    $errors = ApplicationUtils::checkResourcePermissions($GLOBALS['config']);
+
+    if ($errors != array()) {
+        $message = '<p>Insufficient permissions:</p><ul>';
+
+        foreach ($errors as $error) {
+            $message .= '<li>'.$error.'</li>';
+        }
+        $message .= '</ul>';
+
+        header('Content-Type: text/html; charset=utf-8');
+        echo $message;
+        exit;
+    }
+
+    // Display the installation form if no existing config is found
     install();
 }
 
@@ -319,7 +337,7 @@ function checkUpdate()
 function logm($message)
 {
     $t = strval(date('Y/m/d_H:i:s')).' - '.$_SERVER["REMOTE_ADDR"].' - '.strval($message)."\n";
-    file_put_contents($GLOBALS['config']['DATADIR'].'/log.txt',$t,FILE_APPEND);
+    file_put_contents($GLOBAL['config']['LOG_FILE'], $t, FILE_APPEND);
 }
 
 // In a string, converts URLs to clickable links.
@@ -1461,7 +1479,7 @@ function renderPage()
                 $value['tags']=trim(implode(' ',$tags));
                 $LINKSDB[$key]=$value;
             }
-            $LINKSDB->savedb($GLOBALS['config']['PAGECACHE']); // Save to disk.
+            $LINKSDB->savedb($GLOBALS['config']['PAGECACHE']);
             echo '<script>alert("Tag was removed from '.count($linksToAlter).' links.");document.location=\'?\';</script>';
             exit;
         }

diff --git a/tests/ApplicationUtilsTest.php b/tests/ApplicationUtilsTest.php
new file mode 100644 (file)
index 0000000..9a99c6c
--- /dev/null
+++ b/tests/ApplicationUtilsTest.php
@@ -0,0 +1,69 @@
+<?php
+/**
+ * ApplicationUtils' tests
+ */
+
+require_once 'application/ApplicationUtils.php';
+
+
+/**
+ * Unitary tests for Shaarli utilities
+ */
+class ApplicationUtilsTest extends PHPUnit_Framework_TestCase
+{
+    /**
+     * Checks resource permissions for the current Shaarli installation
+     */
+    public function testCheckCurrentResourcePermissions()
+    {
+        $config = array(
+            'CACHEDIR' => 'cache',
+            'CONFIG_FILE' => 'data/config.php',
+            'DATADIR' => 'data',
+            'DATASTORE' => 'data/datastore.php',
+            'IPBANS_FILENAME' => 'data/ipbans.php',
+            'LOG_FILE' => 'data/log.txt',
+            'PAGECACHE' => 'pagecache',
+            'RAINTPL_TMP' => 'tmp',
+            'RAINTPL_TPL' => 'tpl',
+            'UPDATECHECK_FILENAME' => 'data/lastupdatecheck.txt'
+        );
+        $this->assertEquals(
+            array(),
+            ApplicationUtils::checkResourcePermissions($config)
+        );
+    }
+
+    /**
+     * Checks resource permissions for a non-existent Shaarli installation
+     */
+    public function testCheckCurrentResourcePermissionsErrors()
+    {
+        $config = array(
+            'CACHEDIR' => 'null/cache',
+            'CONFIG_FILE' => 'null/data/config.php',
+            'DATADIR' => 'null/data',
+            'DATASTORE' => 'null/data/store.php',
+            'IPBANS_FILENAME' => 'null/data/ipbans.php',
+            'LOG_FILE' => 'null/data/log.txt',
+            'PAGECACHE' => 'null/pagecache',
+            'RAINTPL_TMP' => 'null/tmp',
+            'RAINTPL_TPL' => 'null/tpl',
+            'UPDATECHECK_FILENAME' => 'null/data/lastupdatecheck.txt'
+        );
+        $this->assertEquals(
+            array(
+                '"null/tpl" directory is not readable',
+                '"null/cache" directory is not readable',
+                '"null/cache" directory is not writable',
+                '"null/data" directory is not readable',
+                '"null/data" directory is not writable',
+                '"null/pagecache" directory is not readable',
+                '"null/pagecache" directory is not writable',
+                '"null/tmp" directory is not readable',
+                '"null/tmp" directory is not writable'
+            ),
+            ApplicationUtils::checkResourcePermissions($config)
+        );
+    }
+}

diff --git a/tests/LinkDBTest.php b/tests/LinkDBTest.php
index 451f1d6f8360836b48cee1d173da79cb2c6c858b..8929713d26341941ffce05785d09fe5fb2cced39 100644 (file)
--- a/tests/LinkDBTest.php
+++ b/tests/LinkDBTest.php
@@ -4,6 +4,7 @@
  */
 
 require_once 'application/Cache.php';
+require_once 'application/FileUtils.php';
 require_once 'application/LinkDB.php';
 require_once 'application/Utils.php';
 require_once 'tests/utils/ReferenceLinkDB.php';
@@ -87,8 +88,8 @@ class LinkDBTest extends PHPUnit_Framework_TestCase
     /**
      * Attempt to instantiate a LinkDB whereas the datastore is not writable
      *
-     * @expectedException              PHPUnit_Framework_Error_Warning
-     * @expectedExceptionMessageRegExp /failed to open stream: No such file or directory/
+     * @expectedException              IOException
+     * @expectedExceptionMessageRegExp /Error accessing null/
      */
     public function testConstructDatastoreNotWriteable()
     {