From d6e5f04d3987e498c5cb859eed6bff33d67949df Mon Sep 17 00:00:00 2001 From: ArthurHoaro Date: Sat, 1 Aug 2020 11:10:57 +0200 Subject: [PATCH] Remove anonymous permission and initialize bookmarks on login --- application/bookmark/BookmarkFileService.php | 36 +++++++++---------- application/bookmark/BookmarkIO.php | 8 +++-- application/bookmark/BookmarkInitializer.php | 9 +---- .../bookmark/BookmarkServiceInterface.php | 14 -------- .../DatastoreNotInitializedException.php | 10 ++++++ .../controller/visitor/InstallController.php | 5 --- tests/bookmark/BookmarkInitializerTest.php | 14 +++++--- .../visitor/InstallControllerTest.php | 3 -- 8 files changed, 42 insertions(+), 57 deletions(-) create mode 100644 application/bookmark/exception/DatastoreNotInitializedException.php diff --git a/application/bookmark/BookmarkFileService.php b/application/bookmark/BookmarkFileService.php index 6e04f3b7..b3a90ed4 100644 --- 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 1026e2f9..6bf7f365 100644 --- 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 479ee9a9..cd2d1724 100644 --- 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 37fbda89..ce8bd912 100644 --- 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 index 00000000..f495049d --- /dev/null +++ b/application/bookmark/exception/DatastoreNotInitializedException.php @@ -0,0 +1,10 @@ +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 d23eb069..3906cc7f 100644 --- 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 089c64ac..3b855365 100644 --- 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') -- 2.41.0