Merge pull request #1597 from ArthurHoaro/feature/share-private-bookmark

Feature: Share private bookmarks using a URL containing a private key
2020-10-27 19:40:57 +01:00 · 2020-10-27 19:40:57 +01:00 · 977db7eabc
parent e6215a2ad9 9c04921a8c
commit 977db7eabc
10 changed files with 268 additions and 23 deletions
--- a/application/bookmark/BookmarkFileService.php
+++ b/application/bookmark/BookmarkFileService.php
@ -97,12 +97,15 @@ class BookmarkFileService implements BookmarkServiceInterface
    /**
     * @inheritDoc
     */
-    public function findByHash(string $hash): Bookmark
+    public function findByHash(string $hash, string $privateKey = null): Bookmark
    {
        $bookmark = $this->bookmarkFilter->filter(BookmarkFilter::$FILTER_HASH, $hash);
        // PHP 7.3 introduced array_key_first() to avoid this hack
        $first = reset($bookmark);
-        if (! $this->isLoggedIn && $first->isPrivate()) {
+        if (!$this->isLoggedIn
+            && $first->isPrivate()
+            && (empty($privateKey) || $privateKey !== $first->getAdditionalContentEntry('private_key'))
+        ) {
            throw new Exception('Not authorized');
        }

--- a/application/bookmark/BookmarkServiceInterface.php
+++ b/application/bookmark/BookmarkServiceInterface.php
@ -20,13 +20,14 @@ interface BookmarkServiceInterface
    /**
     * Find a bookmark by hash
     *
-     * @param string $hash
+     * @param string      $hash       Bookmark's hash
+     * @param string|null $privateKey Optional key used to access private links while logged out
     *
     * @return Bookmark
     *
     * @throws \Exception
     */
-    public function findByHash(string $hash): Bookmark;
+    public function findByHash(string $hash, string $privateKey = null);

    /**
     * @param $url
--- a/application/front/controller/admin/ManageShaareController.php
+++ b/application/front/controller/admin/ManageShaareController.php
@ -320,6 +320,32 @@ class ManageShaareController extends ShaarliAdminController
        return $this->redirectFromReferer($request, $response, ['/pin'], ['pin']);
    }

+    /**
+     * GET /admin/shaare/private/{hash} - Attach a private key to given bookmark, then redirect to the sharing URL.
+     */
+    public function sharePrivate(Request $request, Response $response, array $args): Response
+    {
+        $this->checkToken($request);
+
+        $hash = $args['hash'] ?? '';
+        $bookmark = $this->container->bookmarkService->findByHash($hash);
+
+        if ($bookmark->isPrivate() !== true) {
+            return $this->redirect($response, '/shaare/' . $hash);
+        }
+
+        if (empty($bookmark->getAdditionalContentEntry('private_key'))) {
+            $privateKey = bin2hex(random_bytes(16));
+            $bookmark->addAdditionalContentEntry('private_key', $privateKey);
+            $this->container->bookmarkService->set($bookmark);
+        }
+
+        return $this->redirect(
+            $response,
+            '/shaare/' . $hash . '?key=' . $bookmark->getAdditionalContentEntry('private_key')
+        );
+    }
+
    /**
     * Helper function used to display the shaare form whether it's a new or existing bookmark.
     *
--- a/application/front/controller/visitor/BookmarkListController.php
+++ b/application/front/controller/visitor/BookmarkListController.php
@ -137,8 +137,10 @@ class BookmarkListController extends ShaarliVisitorController
     */
    public function permalink(Request $request, Response $response, array $args): Response
    {
+        $privateKey = $request->getParam('key');
+
        try {
-            $bookmark = $this->container->bookmarkService->findByHash($args['hash']);
+            $bookmark = $this->container->bookmarkService->findByHash($args['hash'], $privateKey);
        } catch (BookmarkNotFoundException $e) {
            $this->assignView('error_message', $e->getMessage());

--- a/inc/languages/fr/LC_MESSAGES/shaarli.po
+++ b/inc/languages/fr/LC_MESSAGES/shaarli.po
@ -1,8 +1,8 @@
 msgid ""
 msgstr ""
 "Project-Id-Version: Shaarli\n"
-"POT-Creation-Date: 2020-10-21 15:00+0200\n"
-"PO-Revision-Date: 2020-10-21 15:06+0200\n"
+"POT-Creation-Date: 2020-10-27 19:32+0100\n"
+"PO-Revision-Date: 2020-10-27 19:32+0100\n"
 "Last-Translator: \n"
 "Language-Team: Shaarli\n"
 "Language: fr_FR\n"
@ -123,38 +123,38 @@ msgstr ""
 "l'extension php-gd doit être chargée pour utiliser les miniatures. Les "
 "miniatures sont désormais désactivées. Rechargez la page."

-#: application/Utils.php:383
+#: application/Utils.php:385
 msgid "Setting not set"
 msgstr "Paramètre non défini"

-#: application/Utils.php:390
+#: application/Utils.php:392
 msgid "Unlimited"
 msgstr "Illimité"

-#: application/Utils.php:393
+#: application/Utils.php:395
 msgid "B"
 msgstr "o"

-#: application/Utils.php:393
+#: application/Utils.php:395
 msgid "kiB"
 msgstr "ko"

-#: application/Utils.php:393
+#: application/Utils.php:395
 msgid "MiB"
 msgstr "Mo"

-#: application/Utils.php:393
+#: application/Utils.php:395
 msgid "GiB"
 msgstr "Go"

-#: application/bookmark/BookmarkFileService.php:180
-#: application/bookmark/BookmarkFileService.php:202
-#: application/bookmark/BookmarkFileService.php:224
-#: application/bookmark/BookmarkFileService.php:238
+#: application/bookmark/BookmarkFileService.php:183
+#: application/bookmark/BookmarkFileService.php:205
+#: application/bookmark/BookmarkFileService.php:227
+#: application/bookmark/BookmarkFileService.php:241
 msgid "You're not authorized to alter the datastore"
 msgstr "Vous n'êtes pas autorisé à modifier les données"

-#: application/bookmark/BookmarkFileService.php:205
+#: application/bookmark/BookmarkFileService.php:208
 msgid "This bookmarks already exists"
 msgstr "Ce marque-page existe déjà."

@ -439,12 +439,12 @@ msgstr "ID du lien non valide."
 msgid "Invalid visibility provided."
 msgstr "Visibilité du lien non valide."

-#: application/front/controller/admin/ManageShaareController.php:352
+#: application/front/controller/admin/ManageShaareController.php:378
 #: tmp/linklist.b91ef64efc3688266305ea9b42e5017e.rtpl.php:171
 msgid "Edit"
 msgstr "Modifier"

-#: application/front/controller/admin/ManageShaareController.php:355
+#: application/front/controller/admin/ManageShaareController.php:381
 #: tmp/page.header.b91ef64efc3688266305ea9b42e5017e.rtpl.php:28
 #: tmp/page.header.cedf684561d925457130839629000a81.rtpl.php:28
 msgid "Shaare"
@ -551,7 +551,7 @@ msgstr "Hier"
 msgid "Daily"
 msgstr "Quotidien"

-#: application/front/controller/visitor/ErrorController.php:36
+#: application/front/controller/visitor/ErrorController.php:33
 msgid "An unexpected error occurred."
 msgstr "Une erreur inattendue s'est produite."

@ -604,7 +604,7 @@ msgstr "Permissions insuffisantes :"
 msgid "Login"
 msgstr "Connexion"

-#: application/front/controller/visitor/LoginController.php:78
+#: application/front/controller/visitor/LoginController.php:77
 msgid "Wrong login/password."
 msgstr "Nom d'utilisateur ou mot de passe incorrect(s)."

@ -738,7 +738,7 @@ msgstr "Impossible de purger %s : le répertoire n'existe pas"
 msgid "An error occurred while running the update "
 msgstr "Une erreur s'est produite lors de l'exécution de la mise à jour "

-#: index.php:65
+#: index.php:80
 msgid "Shared bookmarks on "
 msgstr "Liens partagés sur "

@ -1376,6 +1376,10 @@ msgstr "Changer statut épinglé"
 msgid "Sticky"
 msgstr "Épinglé"

+#: tmp/linklist.b91ef64efc3688266305ea9b42e5017e.rtpl.php:189
+msgid "Share a private link"
+msgstr "Partager un lien privé"
+
 #: tmp/linklist.paging.b91ef64efc3688266305ea9b42e5017e.rtpl.php:5
 #: tmp/linklist.paging.cedf684561d925457130839629000a81.rtpl.php:5
 msgid "Filters"
--- a/index.php
+++ b/index.php
@ -128,6 +128,7 @@ $app->group('/admin', function () {
    $this->get('/add-shaare', '\Shaarli\Front\Controller\Admin\ManageShaareController:addShaare');
    $this->get('/shaare', '\Shaarli\Front\Controller\Admin\ManageShaareController:displayCreateForm');
    $this->get('/shaare/{id:[0-9]+}', '\Shaarli\Front\Controller\Admin\ManageShaareController:displayEditForm');
+    $this->get('/shaare/private/{hash}', '\Shaarli\Front\Controller\Admin\ManageShaareController:sharePrivate');
    $this->post('/shaare', '\Shaarli\Front\Controller\Admin\ManageShaareController:save');
    $this->get('/shaare/delete', '\Shaarli\Front\Controller\Admin\ManageShaareController:deleteBookmark');
    $this->get('/shaare/visibility', '\Shaarli\Front\Controller\Admin\ManageShaareController:changeVisibility');
--- a/tests/bookmark/BookmarkFileServiceTest.php
+++ b/tests/bookmark/BookmarkFileServiceTest.php
@ -897,6 +897,37 @@ class BookmarkFileServiceTest extends TestCase
        $this->publicLinkDB->findByHash('');
    }

+    /**
+     * Test filterHash() on a private bookmark while logged out.
+     */
+    public function testFilterHashPrivateWhileLoggedOut()
+    {
+        $this->expectException(\Exception::class);
+        $this->expectExceptionMessage('Not authorized');
+
+        $hash = smallHash('20141125_084734' . 6);
+
+        $this->publicLinkDB->findByHash($hash);
+    }
+
+    /**
+     * Test filterHash() with private key.
+     */
+    public function testFilterHashWithPrivateKey()
+    {
+        $hash = smallHash('20141125_084734' . 6);
+        $privateKey = 'this is usually auto generated';
+
+        $bookmark = $this->privateLinkDB->findByHash($hash);
+        $bookmark->addAdditionalContentEntry('private_key', $privateKey);
+        $this->privateLinkDB->save();
+
+        $this->privateLinkDB = new BookmarkFileService($this->conf, $this->history, $this->mutex, false);
+        $bookmark = $this->privateLinkDB->findByHash($hash, $privateKey);
+
+        static::assertSame(6, $bookmark->getId());
+    }
+
    /**
     * Test linksCountPerTag all tags without filter.
     * Equal occurrences should be sorted alphabetically.
--- a/tests/front/controller/admin/ManageShaareControllerTest/SharePrivateTest.php
+++ b/tests/front/controller/admin/ManageShaareControllerTest/SharePrivateTest.php
@ -0,0 +1,139 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Shaarli\Front\Controller\Admin\ManageShaareControllerTest;
+
+use Shaarli\Bookmark\Bookmark;
+use Shaarli\Front\Controller\Admin\FrontAdminControllerMockHelper;
+use Shaarli\Front\Controller\Admin\ManageShaareController;
+use Shaarli\Http\HttpAccess;
+use Shaarli\TestCase;
+use Slim\Http\Request;
+use Slim\Http\Response;
+
+/**
+ * Test GET /admin/shaare/private/{hash}
+ */
+class SharePrivateTest extends TestCase
+{
+    use FrontAdminControllerMockHelper;
+
+    /** @var ManageShaareController */
+    protected $controller;
+
+    public function setUp(): void
+    {
+        $this->createContainer();
+
+        $this->container->httpAccess = $this->createMock(HttpAccess::class);
+        $this->controller = new ManageShaareController($this->container);
+    }
+
+    /**
+     * Test shaare private with a private bookmark which does not have a key yet.
+     */
+    public function testSharePrivateWithNewPrivateBookmark(): void
+    {
+        $hash = 'abcdcef';
+        $request = $this->createMock(Request::class);
+        $response = new Response();
+
+        $bookmark = (new Bookmark())
+            ->setId(123)
+            ->setUrl('http://domain.tld')
+            ->setTitle('Title 123')
+            ->setPrivate(true)
+        ;
+
+        $this->container->bookmarkService
+            ->expects(static::once())
+            ->method('findByHash')
+            ->with($hash)
+            ->willReturn($bookmark)
+        ;
+        $this->container->bookmarkService
+            ->expects(static::once())
+            ->method('set')
+            ->with($bookmark, true)
+            ->willReturnCallback(function (Bookmark $bookmark): Bookmark {
+                static::assertSame(32, strlen($bookmark->getAdditionalContentEntry('private_key')));
+
+                return $bookmark;
+            })
+        ;
+
+        $result = $this->controller->sharePrivate($request, $response, ['hash' => $hash]);
+
+        static::assertSame(302, $result->getStatusCode());
+        static::assertRegExp('#/subfolder/shaare/' . $hash . '\?key=\w{32}#', $result->getHeaderLine('Location'));
+    }
+
+    /**
+     * Test shaare private with a private bookmark which does already have a key.
+     */
+    public function testSharePrivateWithExistingPrivateBookmark(): void
+    {
+        $hash = 'abcdcef';
+        $existingKey = 'this is a private key';
+        $request = $this->createMock(Request::class);
+        $response = new Response();
+
+        $bookmark = (new Bookmark())
+            ->setId(123)
+            ->setUrl('http://domain.tld')
+            ->setTitle('Title 123')
+            ->setPrivate(true)
+            ->addAdditionalContentEntry('private_key', $existingKey)
+        ;
+
+        $this->container->bookmarkService
+            ->expects(static::once())
+            ->method('findByHash')
+            ->with($hash)
+            ->willReturn($bookmark)
+        ;
+        $this->container->bookmarkService
+            ->expects(static::never())
+            ->method('set')
+        ;
+
+        $result = $this->controller->sharePrivate($request, $response, ['hash' => $hash]);
+
+        static::assertSame(302, $result->getStatusCode());
+        static::assertSame('/subfolder/shaare/' . $hash . '?key=' . $existingKey, $result->getHeaderLine('Location'));
+    }
+
+    /**
+     * Test shaare private with a public bookmark.
+     */
+    public function testSharePrivateWithPublicBookmark(): void
+    {
+        $hash = 'abcdcef';
+        $request = $this->createMock(Request::class);
+        $response = new Response();
+
+        $bookmark = (new Bookmark())
+            ->setId(123)
+            ->setUrl('http://domain.tld')
+            ->setTitle('Title 123')
+            ->setPrivate(false)
+        ;
+
+        $this->container->bookmarkService
+            ->expects(static::once())
+            ->method('findByHash')
+            ->with($hash)
+            ->willReturn($bookmark)
+        ;
+        $this->container->bookmarkService
+            ->expects(static::never())
+            ->method('set')
+        ;
+
+        $result = $this->controller->sharePrivate($request, $response, ['hash' => $hash]);
+
+        static::assertSame(302, $result->getStatusCode());
+        static::assertSame('/subfolder/shaare/' . $hash, $result->getHeaderLine('Location'));
+    }
+}
--- a/tests/front/controller/visitor/BookmarkListControllerTest.php
+++ b/tests/front/controller/visitor/BookmarkListControllerTest.php
@ -291,6 +291,37 @@ class BookmarkListControllerTest extends TestCase
        );
    }

+    /**
+     * Test GET /shaare/{hash}?key={key} - Find a link by hash using a private link.
+     */
+    public function testPermalinkWithPrivateKey(): void
+    {
+        $hash = 'abcdef';
+        $privateKey = 'this is a private key';
+
+        $assignedVariables = [];
+        $this->assignTemplateVars($assignedVariables);
+
+        $request = $this->createMock(Request::class);
+        $request->method('getParam')->willReturnCallback(function (string $key, $default = null) use ($privateKey) {
+            return $key === 'key' ? $privateKey : $default;
+        });
+        $response = new Response();
+
+        $this->container->bookmarkService
+            ->expects(static::once())
+            ->method('findByHash')
+            ->with($hash, $privateKey)
+            ->willReturn((new Bookmark())->setId(123)->setTitle('Title 1')->setUrl('http://url1.tld'))
+        ;
+
+        $result = $this->controller->permalink($request, $response, ['hash' => $hash]);
+
+        static::assertSame(200, $result->getStatusCode());
+        static::assertSame('linklist', (string) $result->getBody());
+        static::assertCount(1, $assignedVariables['links']);
+    }
+
    /**
     * Test getting link list with thumbnail updates.
     *   -> 2 thumbnails update, only 1 datastore write
--- a/tpl/default/linklist.html
+++ b/tpl/default/linklist.html
@ -129,6 +129,7 @@
      {$strAddTag=t('Add tag')}
      {$strToggleSticky=t('Toggle sticky')}
      {$strSticky=t('Sticky')}
+      {$strShaarePrivate=t('Share a private link')}
      {ignore}End of translations{/ignore}
      {loop="links"}
        <div class="anchor" id="{$value.shorturl}"></div>
@ -241,6 +242,12 @@
                  {$strPermalinkLc}
                </a>

+                {if="$is_logged_in && $value.private"}
+                  <a href="{$base_path}/admin/shaare/private/{$value.shorturl}?token={$token}" title="{$strShaarePrivate}">
+                    <i class="fa fa-share-alt"></i>
+                  </a>
+                {/if}
+
                <div class="pure-u-0 pure-u-lg-visible">
                  {if="isset($value.link_plugin)"}
                    &middot;