Fix email shares not being shown to other users

Signed-off-by: Richard Steinmetz <richard@steinmetz.cloud>

diff --git a/apps/sharebymail/lib/ShareByMailProvider.php b/apps/sharebymail/lib/ShareByMailProvider.php
index ccfebe0bdeddd0281e7a3ea604010156fcad8e91..ee14e354c5284850ef2b756a83e42b3985f8e657 100644 (file)
--- a/apps/sharebymail/lib/ShareByMailProvider.php
+++ b/apps/sharebymail/lib/ShareByMailProvider.php
@@ -855,7 +855,7 @@ class ShareByMailProvider implements IShareProvider {
                                        $or1
                                )
                        );
-               } else {
+               } elseif ($node === null) {
                        $qb->andWhere(
                                $qb->expr()->orX(
                                        $qb->expr()->eq('uid_owner', $qb->createNamedParameter($userId)),

diff --git a/tests/lib/Share20/ShareByMailProviderTest.php b/tests/lib/Share20/ShareByMailProviderTest.php
new file mode 100644 (file)
index 0000000..3558be9
--- /dev/null
+++ b/tests/lib/Share20/ShareByMailProviderTest.php
@@ -0,0 +1,312 @@
+<?php
+
+declare(strict_types=1);
+
+/**
+ * @copyright Copyright (c) 2022 Richard Steinmetz <richard@steinmetz.cloud>
+ *
+ * @author Richard Steinmetz <richard@steinmetz.cloud>
+ *
+ * @license AGPL-3.0-or-later
+ *
+ * This code is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License, version 3,
+ * as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License, version 3,
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>
+ *
+ */
+
+namespace Test\Share20;
+
+use OC\Files\Node\Node;
+use OCA\ShareByMail\Settings\SettingsManager;
+use OCA\ShareByMail\ShareByMailProvider;
+use OCP\DB\QueryBuilder\IQueryBuilder;
+use OCP\Defaults;
+use OCP\EventDispatcher\IEventDispatcher;
+use OCP\Files\IRootFolder;
+use OCP\IConfig;
+use OCP\IDBConnection;
+use OCP\IL10N;
+use OCP\ILogger;
+use OCP\IURLGenerator;
+use OCP\IUserManager;
+use OCP\Mail\IMailer;
+use OCP\Security\IHasher;
+use OCP\Security\ISecureRandom;
+use OCP\Share\IShare;
+use PHPUnit\Framework\MockObject\MockObject;
+use Test\TestCase;
+
+/**
+ * Class ShareByMailProviderTest
+ *
+ * @package Test\Share20
+ * @group DB
+ */
+class ShareByMailProviderTest extends TestCase {
+
+       /** @var IDBConnection */
+       protected $dbConn;
+
+       /** @var IUserManager | \PHPUnit\Framework\MockObject\MockObject */
+       protected $userManager;
+
+       /** @var IRootFolder | \PHPUnit\Framework\MockObject\MockObject */
+       protected $rootFolder;
+
+       /** @var ShareByMailProvider */
+       protected $provider;
+
+       /** @var \PHPUnit\Framework\MockObject\MockObject|IMailer */
+       protected $mailer;
+
+       /** @var \PHPUnit\Framework\MockObject\MockObject|IL10N */
+       protected $l10n;
+
+       /** @var \PHPUnit\Framework\MockObject\MockObject|Defaults */
+       protected $defaults;
+
+       /** @var \PHPUnit\Framework\MockObject\MockObject|IURLGenerator */
+       protected $urlGenerator;
+
+       /** @var IConfig|MockObject */
+       protected $config;
+
+       /** @var ILogger|MockObject */
+       private $logger;
+
+       /** @var IHasher|MockObject */
+       private $hasher;
+
+       /** @var \OCP\Activity\IManager|MockObject */
+       private $activityManager;
+
+       /** @var IEventDispatcher|MockObject */
+       private $eventDispatcher;
+
+       /** @var \OCP\Share\IManager|MockObject */
+       private $shareManager;
+
+       /** @var ISecureRandom|MockObject */
+       private $secureRandom;
+
+       /** @var SettingsManager|MockObject */
+       private $settingsManager;
+
+       protected function setUp(): void {
+               $this->dbConn = \OC::$server->getDatabaseConnection();
+               $this->userManager = $this->createMock(IUserManager::class);
+               $this->rootFolder = $this->createMock(IRootFolder::class);
+               $this->mailer = $this->createMock(IMailer::class);
+               $this->l10n = $this->createMock(IL10N::class);
+               $this->defaults = $this->getMockBuilder(Defaults::class)->disableOriginalConstructor()->getMock();
+               $this->urlGenerator = $this->createMock(IURLGenerator::class);
+               $this->logger = $this->createMock(ILogger::class);
+               $this->activityManager = $this->createMock(\OCP\Activity\IManager::class);
+               $this->settingsManager = $this->createMock(SettingsManager::class);
+               $this->hasher = $this->createMock(IHasher::class);
+               $this->eventDispatcher = $this->createMock(IEventDispatcher::class);
+               $this->shareManager = $this->createMock(\OCP\Share\IManager::class);
+               $this->secureRandom = $this->createMock(ISecureRandom::class);
+               $this->config = $this->createMock(IConfig::class);
+
+               // Empty share table
+               $this->dbConn->getQueryBuilder()->delete('share')->execute();
+
+               $this->provider = new ShareByMailProvider(
+                       $this->config,
+                       $this->dbConn,
+                       $this->secureRandom,
+                       $this->userManager,
+                       $this->rootFolder,
+                       $this->l10n,
+                       $this->logger,
+                       $this->mailer,
+                       $this->urlGenerator,
+                       $this->activityManager,
+                       $this->settingsManager,
+                       $this->defaults,
+                       $this->hasher,
+                       $this->eventDispatcher,
+                       $this->shareManager,
+               );
+       }
+
+       protected function tearDown(): void {
+               $this->dbConn->getQueryBuilder()->delete('share')->execute();
+               $this->dbConn->getQueryBuilder()->delete('filecache')->execute();
+               $this->dbConn->getQueryBuilder()->delete('storages')->execute();
+       }
+
+       /**
+        * @param int $shareType
+        * @param string $sharedWith
+        * @param string $sharedBy
+        * @param string $shareOwner
+        * @param string $itemType
+        * @param int $fileSource
+        * @param string $fileTarget
+        * @param int $permissions
+        * @param $token
+        * @param $expiration
+        * @param $parent
+        * @return int
+        *
+        * @throws \OCP\DB\Exception
+        */
+       private function addShareToDB($shareType, $sharedWith, $sharedBy, $shareOwner,
+                       $itemType, $fileSource, $fileTarget, $permissions, $token, $expiration,
+                       $parent) {
+               $qb = $this->dbConn->getQueryBuilder();
+               $qb->insert('share');
+
+               if ($shareType) {
+                       $qb->setValue('share_type', $qb->expr()->literal($shareType));
+               }
+               if ($sharedWith) {
+                       $qb->setValue('share_with', $qb->expr()->literal($sharedWith));
+               }
+               if ($sharedBy) {
+                       $qb->setValue('uid_initiator', $qb->expr()->literal($sharedBy));
+               }
+               if ($shareOwner) {
+                       $qb->setValue('uid_owner', $qb->expr()->literal($shareOwner));
+               }
+               if ($itemType) {
+                       $qb->setValue('item_type', $qb->expr()->literal($itemType));
+               }
+               if ($fileSource) {
+                       $qb->setValue('file_source', $qb->expr()->literal($fileSource));
+               }
+               if ($fileTarget) {
+                       $qb->setValue('file_target', $qb->expr()->literal($fileTarget));
+               }
+               if ($permissions) {
+                       $qb->setValue('permissions', $qb->expr()->literal($permissions));
+               }
+               if ($token) {
+                       $qb->setValue('token', $qb->expr()->literal($token));
+               }
+               if ($expiration) {
+                       $qb->setValue('expiration', $qb->createNamedParameter($expiration, IQueryBuilder::PARAM_DATE));
+               }
+               if ($parent) {
+                       $qb->setValue('parent', $qb->expr()->literal($parent));
+               }
+
+               $this->assertEquals(1, $qb->execute());
+               return $qb->getLastInsertId();
+       }
+
+       public function testGetSharesByWithResharesAndNoNode() {
+               $this->addShareToDB(
+                       IShare::TYPE_EMAIL,
+                       'external.one@domain.tld',
+                       'user1',
+                       'user1',
+                       'folder',
+                       42,
+                       null,
+                       17,
+                       'foobar',
+                       null,
+                       null,
+               );
+               $this->addShareToDB(
+                       IShare::TYPE_EMAIL,
+                       'external.two@domain.tld',
+                       'user2',
+                       'user2',
+                       'folder',
+                       42,
+                       null,
+                       17,
+                       'barfoo',
+                       null,
+                       null,
+               );
+
+               // Return own shares only if not asked for a specific node
+               /** @var IShare[] $actual */
+               $actual = $this->provider->getSharesBy(
+                       'user1',
+                       IShare::TYPE_EMAIL,
+                       null,
+                       true,
+                       -1,
+                       0,
+               );
+
+               $this->assertCount(1, $actual);
+
+               $this->assertEquals(IShare::TYPE_EMAIL, $actual[0]->getShareType());
+               $this->assertEquals('user1', $actual[0]->getSharedBy());
+               $this->assertEquals('user1', $actual[0]->getShareOwner());
+               $this->assertEquals('external.one@domain.tld', $actual[0]->getSharedWith());
+       }
+
+       public function testGetSharesByWithResharesAndNode() {
+               $this->addShareToDB(
+                       IShare::TYPE_EMAIL,
+                       'external.one@domain.tld',
+                       'user1',
+                       'user1',
+                       'folder',
+                       42,
+                       null,
+                       17,
+                       'foobar',
+                       null,
+                       null,
+               );
+               $this->addShareToDB(
+                       IShare::TYPE_EMAIL,
+                       'external.two@domain.tld',
+                       'user2',
+                       'user2',
+                       'folder',
+                       42,
+                       null,
+                       17,
+                       'barfoo',
+                       null,
+                       null,
+               );
+
+               $node = $this->createMock(Node::class);
+               $node->expects($this->once())
+                       ->method('getId')
+                       ->willReturn(42);
+
+               // Return all shares if asked for specific node
+               /** @var IShare[] $actual */
+               $actual = $this->provider->getSharesBy(
+                       'user1',
+                       IShare::TYPE_EMAIL,
+                       $node,
+                       true,
+                       -1,
+                       0,
+               );
+
+               $this->assertCount(2, $actual);
+
+               $this->assertEquals(IShare::TYPE_EMAIL, $actual[0]->getShareType());
+               $this->assertEquals('user1', $actual[0]->getSharedBy());
+               $this->assertEquals('user1', $actual[0]->getShareOwner());
+               $this->assertEquals('external.one@domain.tld', $actual[0]->getSharedWith());
+
+               $this->assertEquals(IShare::TYPE_EMAIL, $actual[1]->getShareType());
+               $this->assertEquals('user2', $actual[1]->getSharedBy());
+               $this->assertEquals('user2', $actual[1]->getShareOwner());
+               $this->assertEquals('external.two@domain.tld', $actual[1]->getSharedWith());
+       }
+}