Remove anonymous permission and initialize bookmarks on login

diff --git a/application/bookmark/BookmarkFileService.php b/application/bookmark/BookmarkFileService.php
index 6e04f3b71cee32ea88720a16f6f6b99bad7b4e91..b3a90ed4623beecc7c75d96be635e33decfd6aed 100644 (file)
--- a/application/bookmark/BookmarkFileService.php
+++ b/application/bookmark/BookmarkFileService.php
@@ -6,6 +6,7 @@ namespace Shaarli\Bookmark;
 
 use Exception;
 use Shaarli\Bookmark\Exception\BookmarkNotFoundException;
+use Shaarli\Bookmark\Exception\DatastoreNotInitializedException;
 use Shaarli\Bookmark\Exception\EmptyDataStoreException;
 use Shaarli\Config\ConfigManager;
 use Shaarli\Formatter\BookmarkMarkdownFormatter;
@@ -46,9 +47,6 @@ class BookmarkFileService implements BookmarkServiceInterface
     /** @var bool true for logged in users. Default value to retrieve private bookmarks. */
     protected $isLoggedIn;
 
-    /** @var bool Allow datastore alteration from not logged in users. */
-    protected $anonymousPermission = false;
-
     /**
      * @inheritDoc
      */
@@ -65,10 +63,16 @@ class BookmarkFileService implements BookmarkServiceInterface
         } else {
             try {
                 $this->bookmarks = $this->bookmarksIO->read();
-            } catch (EmptyDataStoreException $e) {
+            } catch (EmptyDataStoreException|DatastoreNotInitializedException $e) {
                 $this->bookmarks = new BookmarkArray();
+
                 if ($this->isLoggedIn) {
-                    $this->save();
+                    // Datastore file does not exists, we initialize it with default bookmarks.
+                    if ($e instanceof DatastoreNotInitializedException) {
+                        $this->initialize();
+                    } else {
+                        $this->save();
+                    }
                 }
             }
 
@@ -157,7 +161,7 @@ class BookmarkFileService implements BookmarkServiceInterface
      */
     public function set($bookmark, $save = true)
     {
-        if (true !== $this->isLoggedIn && true !== $this->anonymousPermission) {
+        if (true !== $this->isLoggedIn) {
             throw new Exception(t('You\'re not authorized to alter the datastore'));
         }
         if (! $bookmark instanceof Bookmark) {
@@ -182,7 +186,7 @@ class BookmarkFileService implements BookmarkServiceInterface
      */
     public function add($bookmark, $save = true)
     {
-        if (true !== $this->isLoggedIn && true !== $this->anonymousPermission) {
+        if (true !== $this->isLoggedIn) {
             throw new Exception(t('You\'re not authorized to alter the datastore'));
         }
         if (! $bookmark instanceof Bookmark) {
@@ -207,7 +211,7 @@ class BookmarkFileService implements BookmarkServiceInterface
      */
     public function addOrSet($bookmark, $save = true)
     {
-        if (true !== $this->isLoggedIn && true !== $this->anonymousPermission) {
+        if (true !== $this->isLoggedIn) {
             throw new Exception(t('You\'re not authorized to alter the datastore'));
         }
         if (! $bookmark instanceof Bookmark) {
@@ -224,7 +228,7 @@ class BookmarkFileService implements BookmarkServiceInterface
      */
     public function remove($bookmark, $save = true)
     {
-        if (true !== $this->isLoggedIn && true !== $this->anonymousPermission) {
+        if (true !== $this->isLoggedIn) {
             throw new Exception(t('You\'re not authorized to alter the datastore'));
         }
         if (! $bookmark instanceof Bookmark) {
@@ -277,7 +281,7 @@ class BookmarkFileService implements BookmarkServiceInterface
      */
     public function save()
     {
-        if (true !== $this->isLoggedIn && true !== $this->anonymousPermission) {
+        if (true !== $this->isLoggedIn) {
             // TODO: raise an Exception instead
             die('You are not authorized to change the database.');
         }
@@ -359,16 +363,10 @@ class BookmarkFileService implements BookmarkServiceInterface
     {
         $initializer = new BookmarkInitializer($this);
         $initializer->initialize();
-    }
 
-    public function enableAnonymousPermission(): void
-    {
-        $this->anonymousPermission = true;
-    }
-
-    public function disableAnonymousPermission(): void
-    {
-        $this->anonymousPermission = false;
+        if (true === $this->isLoggedIn) {
+            $this->save();
+        }
     }
 
     /**

diff --git a/application/bookmark/BookmarkIO.php b/application/bookmark/BookmarkIO.php
index 1026e2f978d0a43650f0753e23a6b92529b1f91d..6bf7f3654ebfdd41f87b9468f00c135c2725e9da 100644 (file)
--- a/application/bookmark/BookmarkIO.php
+++ b/application/bookmark/BookmarkIO.php
@@ -2,6 +2,7 @@
 
 namespace Shaarli\Bookmark;
 
+use Shaarli\Bookmark\Exception\DatastoreNotInitializedException;
 use Shaarli\Bookmark\Exception\EmptyDataStoreException;
 use Shaarli\Bookmark\Exception\NotWritableDataStoreException;
 use Shaarli\Config\ConfigManager;
@@ -52,13 +53,14 @@ class BookmarkIO
      *
      * @return BookmarkArray instance
      *
-     * @throws NotWritableDataStoreException Data couldn't be loaded
-     * @throws EmptyDataStoreException       Datastore doesn't exist
+     * @throws NotWritableDataStoreException    Data couldn't be loaded
+     * @throws EmptyDataStoreException          Datastore file exists but does not contain any bookmark
+     * @throws DatastoreNotInitializedException File does not exists
      */
     public function read()
     {
         if (! file_exists($this->datastore)) {
-            throw new EmptyDataStoreException();
+            throw new DatastoreNotInitializedException();
         }
 
         if (!is_writable($this->datastore)) {

diff --git a/application/bookmark/BookmarkInitializer.php b/application/bookmark/BookmarkInitializer.php
index 479ee9a9e749f195e0e7a9c89cbabbc45f736aa6..cd2d1724c1ad088193ffe51c1bde44a7d4ab2c2b 100644 (file)
--- a/application/bookmark/BookmarkInitializer.php
+++ b/application/bookmark/BookmarkInitializer.php
@@ -6,8 +6,7 @@ namespace Shaarli\Bookmark;
  * Class BookmarkInitializer
  *
  * This class is used to initialized default bookmarks after a fresh install of Shaarli.
- * It is no longer call when the data store is empty,
- * because user might want to delete default bookmarks after the install.
+ * It should be only called if the datastore file does not exist(users might want to delete the default bookmarks).
  *
  * To prevent data corruption, it does not overwrite existing bookmarks,
  * even though there should not be any.
@@ -34,8 +33,6 @@ class BookmarkInitializer
      */
     public function initialize()
     {
-        $this->bookmarkService->enableAnonymousPermission();
-
         $bookmark = new Bookmark();
         $bookmark->setTitle(t('My secret stuff... - Pastebin.com'));
         $bookmark->setUrl('http://sebsauvage.net/paste/?8434b27936c09649#bR7XsXhoTiLcqCpQbmOpBi3rq2zzQUC5hBI7ZT1O3x8=');
@@ -57,9 +54,5 @@ You use the community supported version of the original Shaarli project, by Seba
         ));
         $bookmark->setTagsString('opensource software');
         $this->bookmarkService->add($bookmark, false);
-
-        $this->bookmarkService->save();
-
-        $this->bookmarkService->disableAnonymousPermission();
     }
 }

diff --git a/application/bookmark/BookmarkServiceInterface.php b/application/bookmark/BookmarkServiceInterface.php
index 37fbda890e2d607552d9b0620ac66f3b4a57cae0..ce8bd912bf6b5a8d086646a9f1fd4f8fd297fe62 100644 (file)
--- a/application/bookmark/BookmarkServiceInterface.php
+++ b/application/bookmark/BookmarkServiceInterface.php
@@ -6,7 +6,6 @@ namespace Shaarli\Bookmark;
 use Shaarli\Bookmark\Exception\BookmarkNotFoundException;
 use Shaarli\Bookmark\Exception\NotWritableDataStoreException;
 use Shaarli\Config\ConfigManager;
-use Shaarli\Exceptions\IOException;
 use Shaarli\History;
 
 /**
@@ -177,17 +176,4 @@ interface BookmarkServiceInterface
      * Creates the default database after a fresh install.
      */
     public function initialize();
-
-    /**
-     * Allow to write the datastore from anonymous session (not logged in).
-     *
-     * This covers a few specific use cases, such as datastore initialization,
-     * but it should be used carefully as it can lead to security issues.
-     */
-    public function enableAnonymousPermission();
-
-    /**
-     * Disable anonymous permission.
-     */
-    public function disableAnonymousPermission();
 }

diff --git a/application/bookmark/exception/DatastoreNotInitializedException.php b/application/bookmark/exception/DatastoreNotInitializedException.php
new file mode 100644 (file)
index 0000000..f495049
--- /dev/null
+++ b/application/bookmark/exception/DatastoreNotInitializedException.php
@@ -0,0 +1,10 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Shaarli\Bookmark\Exception;
+
+class DatastoreNotInitializedException extends \Exception
+{
+
+}

diff --git a/application/front/controller/visitor/InstallController.php b/application/front/controller/visitor/InstallController.php
index 5e3152c7b6ab52951d28db67b90cdb2ddf801500..7cb3277794fbe8c9b2929766b68cc9bcfad21c5b 100644 (file)
--- a/application/front/controller/visitor/InstallController.php
+++ b/application/front/controller/visitor/InstallController.php
@@ -5,7 +5,6 @@ declare(strict_types=1);
 namespace Shaarli\Front\Controller\Visitor;
 
 use Shaarli\ApplicationUtils;
-use Shaarli\Bookmark\BookmarkFilter;
 use Shaarli\Container\ShaarliContainer;
 use Shaarli\Front\Exception\AlreadyInstalledException;
 use Shaarli\Front\Exception\ResourcePermissionException;
@@ -140,10 +139,6 @@ class InstallController extends ShaarliVisitorController
             return $response->write($this->render('error'));
         }
 
-        if ($this->container->bookmarkService->count(BookmarkFilter::$ALL) === 0) {
-            $this->container->bookmarkService->initialize();
-        }
-
         $this->container->sessionManager->setSessionParameter(
             SessionManager::KEY_SUCCESS_MESSAGES,
             [t('Shaarli is now configured. Please login and start shaaring your bookmarks!')]

diff --git a/tests/bookmark/BookmarkInitializerTest.php b/tests/bookmark/BookmarkInitializerTest.php
index d23eb0695b1fdde093b4fcb4b6274a8bd44cb44a..3906cc7f4e3f542531eaa0b53b3b174c07cfc18a 100644 (file)
--- a/tests/bookmark/BookmarkInitializerTest.php
+++ b/tests/bookmark/BookmarkInitializerTest.php
@@ -3,7 +3,6 @@
 namespace Shaarli\Bookmark;
 
 use PHPUnit\Framework\TestCase;
-use ReferenceLinkDB;
 use Shaarli\Config\ConfigManager;
 use Shaarli\History;
 
@@ -54,9 +53,9 @@ class BookmarkInitializerTest extends TestCase
     }
 
     /**
-     * Test initialize() with an empty data store.
+     * Test initialize() with a data store containing bookmarks.
      */
-    public function testInitializeEmptyDataStore()
+    public function testInitializeNotEmptyDataStore(): void
     {
         $refDB = new \ReferenceLinkDB();
         $refDB->write(self::$testDatastore);
@@ -79,6 +78,8 @@ class BookmarkInitializerTest extends TestCase
         );
         $this->assertFalse($bookmark->isPrivate());
 
+        $this->bookmarkService->save();
+
         // Reload from file
         $this->bookmarkService = new BookmarkFileService($this->conf, $this->history, true);
         $this->assertEquals($refDB->countLinks() + 2, $this->bookmarkService->count());
@@ -97,10 +98,13 @@ class BookmarkInitializerTest extends TestCase
     }
 
     /**
-     * Test initialize() with a data store containing bookmarks.
+     * Test initialize() with an a non existent datastore file .
      */
-    public function testInitializeNotEmptyDataStore()
+    public function testInitializeNonExistentDataStore(): void
     {
+        $this->conf->set('resource.datastore', static::$testDatastore . '_empty');
+        $this->bookmarkService = new BookmarkFileService($this->conf, $this->history, true);
+
         $this->initializer->initialize();
 
         $this->assertEquals(2, $this->bookmarkService->count());

diff --git a/tests/front/controller/visitor/InstallControllerTest.php b/tests/front/controller/visitor/InstallControllerTest.php
index 089c64acd05a8c0b2fc663efa5cda41612aa328f..3b855365024a9e9e8462299ed5e1e85a74449aaf 100644 (file)
--- a/tests/front/controller/visitor/InstallControllerTest.php
+++ b/tests/front/controller/visitor/InstallControllerTest.php
@@ -224,9 +224,6 @@ class InstallControllerTest extends TestCase
         ;
         $this->container->conf->expects(static::once())->method('write');
 
-        $this->container->bookmarkService->expects(static::once())->method('count')->willReturn(0);
-        $this->container->bookmarkService->expects(static::once())->method('initialize');
-
         $this->container->sessionManager
             ->expects(static::once())
             ->method('setSessionParameter')