Multiple fixes

- Fix tests
- Use non deprecated event stuff
- Add a bit of type hinting to the new stuff
- More safe handling of instanceOfStorage (share might not be the first
  wrapper)
- Fix resharing

Signed-off-by: Carl Schwan <carl@carlschwan.eu>

apps/dav/lib/Connector/Sabre/Node.php

@@ -38,6 +38,7 @@ namespace OCA\DAV\Connector\Sabre;
 use OC\Files\Mount\MoveableMount;
 use OC\Files\Node\File;
 use OC\Files\Node\Folder;
+use OC\Files\Storage\Wrapper\Wrapper;
 use OC\Files\View;
 use OCA\DAV\Connector\Sabre\Exception\InvalidPath;
 use OCP\Files\FileInfo;
@@ -334,9 +335,14 @@ abstract class Node implements \Sabre\DAV\INode {
 			$storage = null;
 		}
 
-		if ($storage && $storage->instanceOfStorage('\OCA\Files_Sharing\SharedStorage')) {
+		if ($storage && $storage->instanceOfStorage(\OCA\Files_Sharing\SharedStorage::class)) {
 			/** @var \OCA\Files_Sharing\SharedStorage $storage */
-			$attributes = $storage->getShare()->getAttributes()->toArray();
+			$attributes = $storage->getShare()->getAttributes();
+			if ($attributes === null) {
+				return [];
+			} else {
+				return $attributes->toArray();
+			}
 		}
 
 		return $attributes;

apps/dav/lib/Connector/Sabre/ServerFactory.php

@@ -45,6 +45,7 @@ use OCP\IRequest;
 use OCP\ITagManager;
 use OCP\IUserSession;
 use OCP\SabrePluginEvent;
+use Psr\Log\LoggerInterface;
 use Sabre\DAV\Auth\Plugin;
 use Symfony\Component\EventDispatcher\EventDispatcherInterface;
 
@@ -53,6 +54,8 @@ class ServerFactory {
 	private $config;
 	/** @var ILogger */
 	private $logger;
+	/** @var LoggerInterface */
+	private $psrLogger;
 	/** @var IDBConnection */
 	private $databaseConnection;
 	/** @var IUserSession */
@@ -73,6 +76,7 @@ class ServerFactory {
 	/**
 	 * @param IConfig $config
 	 * @param ILogger $logger
+	 * @param LoggerInterface $psrLogger
 	 * @param IDBConnection $databaseConnection
 	 * @param IUserSession $userSession
 	 * @param IMountManager $mountManager
@@ -83,6 +87,7 @@ class ServerFactory {
 	public function __construct(
 		IConfig $config,
 		ILogger $logger,
+		LoggerInterface $psrLogger,
 		IDBConnection $databaseConnection,
 		IUserSession $userSession,
 		IMountManager $mountManager,
@@ -94,6 +99,7 @@ class ServerFactory {
 	) {
 		$this->config = $config;
 		$this->logger = $logger;
+		$this->psrLogger = $psrLogger;
 		$this->databaseConnection = $databaseConnection;
 		$this->userSession = $userSession;
 		$this->mountManager = $mountManager;
@@ -185,7 +191,7 @@ class ServerFactory {
 
 			// Allow view-only plugin for webdav requests
 			$server->addPlugin(new ViewOnlyPlugin(
-				$this->logger
+				$this->psrLogger
 			));
 
 			if ($this->userSession->isLoggedIn()) {

apps/dav/lib/Controller/DirectController.php

@@ -36,6 +36,7 @@ use OCP\AppFramework\OCSController;
 use OCP\AppFramework\Utility\ITimeFactory;
 use OCP\EventDispatcher\GenericEvent;
 use OCP\EventDispatcher\IEventDispatcher;
+use OCP\Files\Events\BeforeDirectFileDownloadEvent;
 use OCP\Files\File;
 use OCP\Files\IRootFolder;
 use OCP\IRequest;
@@ -106,10 +107,10 @@ class DirectController extends OCSController {
 			throw new OCSBadRequestException('Direct download only works for files');
 		}
 
-		$event = new GenericEvent(null, ['path' => $userFolder->getRelativePath($file->getPath())]);
-		$this->eventDispatcher->dispatch('file.beforeGetDirect', $event);
+		$event = new BeforeDirectFileDownloadEvent($userFolder->getRelativePath($file->getPath()));
+		$this->eventDispatcher->dispatchTyped($event);
 
-		if ($event->getArgument('run') === false) {
+		if ($event->isSuccessful() === false) {
 			throw new OCSForbiddenException('Permission denied to download file');
 		}
 

apps/dav/lib/DAV/ViewOnlyPlugin.php

@@ -37,18 +37,11 @@ use Sabre\DAV\Exception\NotFound;
  */
 class ViewOnlyPlugin extends ServerPlugin {
 
-	/** @var Server $server */
-	private $server;
+	private ?Server $server = null;
+	private LoggerInterface $logger;
 
-	/** @var LoggerInterface $logger */
-	private $logger;
-
-	/**
-	 * @param LoggerInterface $logger
-	 */
 	public function __construct(LoggerInterface $logger) {
 		$this->logger = $logger;
-		$this->server = null;
 	}
 
 	/**
@@ -58,11 +51,8 @@ class ViewOnlyPlugin extends ServerPlugin {
 	 * addPlugin is called.
 	 *
 	 * This method should set up the required event subscriptions.
-	 *
-	 * @param Server $server
-	 * @return void
 	 */
-	public function initialize(Server $server) {
+	public function initialize(Server $server): void {
 		$this->server = $server;
 		//priority 90 to make sure the plugin is called before
 		//Sabre\DAV\CorePlugin::httpGet
@@ -73,17 +63,14 @@ class ViewOnlyPlugin extends ServerPlugin {
 	 * Disallow download via DAV Api in case file being received share
 	 * and having special permission
 	 *
-	 * @param RequestInterface $request request object
-	 * @return boolean
 	 * @throws Forbidden
 	 * @throws NotFoundException
 	 */
-	public function checkViewOnly(
-		RequestInterface $request
-	) {
+	public function checkViewOnly(RequestInterface $request): bool {
 		$path = $request->getPath();
 
 		try {
+			assert($this->server !== null);
 			$davNode = $this->server->tree->getNodeForPath($path);
 			if (!($davNode instanceof DavFile)) {
 				return true;
@@ -92,21 +79,28 @@ class ViewOnlyPlugin extends ServerPlugin {
 			$node = $davNode->getNode();
 
 			$storage = $node->getStorage();
-			// using string as we have no guarantee that "files_sharing" app is loaded
-			if (!$storage->instanceOfStorage('OCA\Files_Sharing\SharedStorage')) {
+
+			if (!$storage->instanceOfStorage(\OCA\Files_Sharing\SharedStorage::class)) {
 				return true;
 			}
 			// Extract extra permissions
 			/** @var \OCA\Files_Sharing\SharedStorage $storage */
 			$share = $storage->getShare();
 
+			$attributes = $share->getAttributes();
+			if ($attributes === null) {
+				return true;
+			}
+
 			// Check if read-only and on whether permission can download is both set and disabled.
-			$canDownload = $share->getAttributes()->getAttribute('permissions', 'download');
+			$canDownload = $attributes->getAttribute('permissions', 'download');
 			if ($canDownload !== null && !$canDownload) {
 				throw new Forbidden('Access to this resource has been denied because it is in view-only mode.');
 			}
 		} catch (NotFound $e) {
-			$this->logger->warning($e->getMessage());
+			$this->logger->warning($e->getMessage(), [
+				'exception' => $e,
+			]);
 		}
 
 		return true;

apps/dav/lib/Server.php

@@ -232,7 +232,7 @@ class Server {
 
 		// Allow view-only plugin for webdav requests
 		$this->server->addPlugin(new ViewOnlyPlugin(
-			$logger
+			\OC::$server->get(LoggerInterface::class)
 		));
 
 		if (BrowserErrorPagePlugin::isBrowserRequest($request)) {

apps/dav/tests/unit/Controller/DirectControllerTest.php

@@ -34,11 +34,12 @@ use OCP\AppFramework\Http\DataResponse;
 use OCP\AppFramework\OCS\OCSBadRequestException;
 use OCP\AppFramework\OCS\OCSNotFoundException;
 use OCP\AppFramework\Utility\ITimeFactory;
+use OCP\EventDispatcher\IEventDispatcher;
 use OCP\Files\File;
 use OCP\Files\Folder;
 use OCP\Files\IRootFolder;
 use OCP\IRequest;
-use OCP\IURLGenerator;
+use OCP\IUrlGenerator;
 use OCP\Security\ISecureRandom;
 use Test\TestCase;
 
@@ -56,11 +57,13 @@ class DirectControllerTest extends TestCase {
 	/** @var ITimeFactory|\PHPUnit\Framework\MockObject\MockObject */
 	private $timeFactory;
 
-	/** @var IURLGenerator|\PHPUnit\Framework\MockObject\MockObject */
+	/** @var IUrlGenerator|\PHPUnit\Framework\MockObject\MockObject */
 	private $urlGenerator;
 
-	/** @var DirectController */
-	private $controller;
+	/** @var IEventDispatcher|\PHPUnit\Framework\MockObject\MockObject */
+	private $eventDispatcher;
+
+	private DirectController $controller;
 
 	protected function setUp(): void {
 		parent::setUp();
@@ -69,7 +72,8 @@ class DirectControllerTest extends TestCase {
 		$this->directMapper = $this->createMock(DirectMapper::class);
 		$this->random = $this->createMock(ISecureRandom::class);
 		$this->timeFactory = $this->createMock(ITimeFactory::class);
-		$this->urlGenerator = $this->createMock(IURLGenerator::class);
+		$this->urlGenerator = $this->createMock(IUrlGenerator::class);
+		$this->eventDispatcher = $this->createMock(IEventDispatcher::class);
 
 		$this->controller = new DirectController(
 			'dav',
@@ -79,11 +83,12 @@ class DirectControllerTest extends TestCase {
 			$this->directMapper,
 			$this->random,
 			$this->timeFactory,
-			$this->urlGenerator
+			$this->urlGenerator,
+			$this->eventDispatcher
 		);
 	}
 
-	public function testGetUrlNonExistingFileId() {
+	public function testGetUrlNonExistingFileId(): void {
 		$userFolder = $this->createMock(Folder::class);
 		$this->rootFolder->method('getUserFolder')
 			->with('awesomeUser')
@@ -97,7 +102,7 @@ class DirectControllerTest extends TestCase {
 		$this->controller->getUrl(101);
 	}
 
-	public function testGetUrlForFolder() {
+	public function testGetUrlForFolder(): void {
 		$userFolder = $this->createMock(Folder::class);
 		$this->rootFolder->method('getUserFolder')
 			->with('awesomeUser')
@@ -113,7 +118,7 @@ class DirectControllerTest extends TestCase {
 		$this->controller->getUrl(101);
 	}
 
-	public function testGetUrlValid() {
+	public function testGetUrlValid(): void {
 		$userFolder = $this->createMock(Folder::class);
 		$this->rootFolder->method('getUserFolder')
 			->with('awesomeUser')
@@ -128,6 +133,9 @@ class DirectControllerTest extends TestCase {
 			->with(101)
 			->willReturn([$file]);
 
+		$userFolder->method('getRelativePath')
+			->willReturn('/path');
+
 		$this->random->method('generate')
 			->with(
 				60,

apps/dav/tests/unit/DAV/ViewOnlyPluginTest.php

@@ -36,8 +36,7 @@ use OCA\DAV\Connector\Sabre\Exception\Forbidden;
 
 class ViewOnlyPluginTest extends TestCase {
 
-	/** @var ViewOnlyPlugin */
-	private $plugin;
+	private ViewOnlyPlugin $plugin;
 	/** @var Tree | \PHPUnit\Framework\MockObject\MockObject */
 	private $tree;
 	/** @var RequestInterface | \PHPUnit\Framework\MockObject\MockObject */
@@ -56,14 +55,14 @@ class ViewOnlyPluginTest extends TestCase {
 		$this->plugin->initialize($server);
 	}
 
-	public function testCanGetNonDav() {
+	public function testCanGetNonDav(): void {
 		$this->request->expects($this->once())->method('getPath')->willReturn('files/test/target');
 		$this->tree->method('getNodeForPath')->willReturn(null);
 
 		$this->assertTrue($this->plugin->checkViewOnly($this->request));
 	}
 
-	public function testCanGetNonShared() {
+	public function testCanGetNonShared(): void {
 		$this->request->expects($this->once())->method('getPath')->willReturn('files/test/target');
 		$davNode = $this->createMock(DavFile::class);
 		$this->tree->method('getNodeForPath')->willReturn($davNode);
@@ -78,7 +77,7 @@ class ViewOnlyPluginTest extends TestCase {
 		$this->assertTrue($this->plugin->checkViewOnly($this->request));
 	}
 
-	public function providesDataForCanGet() {
+	public function providesDataForCanGet(): array {
 		return [
 			// has attribute permissions-download enabled - can get file
 			[ $this->createMock(File::class), true, true],
@@ -92,7 +91,7 @@ class ViewOnlyPluginTest extends TestCase {
 	/**
 	 * @dataProvider providesDataForCanGet
 	 */
-	public function testCanGet($nodeInfo, $attrEnabled, $expectCanDownloadFile) {
+	public function testCanGet(File $nodeInfo, ?bool $attrEnabled, bool $expectCanDownloadFile): void {
 		$this->request->expects($this->once())->method('getPath')->willReturn('files/test/target');
 
 		$davNode = $this->createMock(DavFile::class);

apps/files_sharing/lib/AppInfo/Application.php

@@ -50,6 +50,7 @@ use OCA\Files_Sharing\Notification\Listener;
 use OCA\Files_Sharing\Notification\Notifier;
 use OCA\Files\Event\LoadAdditionalScriptsEvent;
 use OCA\Files\Event\LoadSidebar;
+use OCP\Files\Event\BeforeDirectGetEvent;
 use OCA\Files_Sharing\ShareBackend\File;
 use OCA\Files_Sharing\ShareBackend\Folder;
 use OCA\Files_Sharing\ViewOnly;
@@ -61,6 +62,8 @@ use OCP\EventDispatcher\IEventDispatcher;
 use OCP\EventDispatcher\GenericEvent;
 use OCP\Federation\ICloudIdManager;
 use OCP\Files\Config\IMountProviderCollection;
+use OCP\Files\Events\BeforeDirectFileDownloadEvent;
+use OCP\Files\Events\BeforeZipCreatedEvent;
 use OCP\Files\IRootFolder;
 use OCP\Group\Events\UserAddedEvent;
 use OCP\IDBConnection;
@@ -156,59 +159,53 @@ class Application extends App implements IBootstrap {
 
 	public function registerDownloadEvents(
 		IEventDispatcher $dispatcher,
-		?IUserSession $userSession,
+		IUserSession $userSession,
 		IRootFolder $rootFolder
 	): void {
 
 		$dispatcher->addListener(
-			'file.beforeGetDirect',
-			function (GenericEvent $event) use ($userSession, $rootFolder) {
-				$pathsToCheck = [$event->getArgument('path')];
-				$event->setArgument('run', true);
-
+			BeforeDirectFileDownloadEvent::class,
+			function (BeforeDirectFileDownloadEvent $event) use ($userSession, $rootFolder): void {
+				$pathsToCheck = [$event->getPath()];
 				// Check only for user/group shares. Don't restrict e.g. share links
-				if ($userSession && $userSession->isLoggedIn()) {
-					$uid = $userSession->getUser()->getUID();
+				$user = $userSession->getUser();
+				if ($user) {
 					$viewOnlyHandler = new ViewOnly(
-						$rootFolder->getUserFolder($uid)
+						$rootFolder->getUserFolder($user->getUID())
 					);
 					if (!$viewOnlyHandler->check($pathsToCheck)) {
-						$event->setArgument('run', false);
-						$event->setArgument('errorMessage', 'Access to this resource or one of its sub-items has been denied.');
+						$event->setSuccessful(false);
+						$event->setErrorMessage('Access to this resource or one of its sub-items has been denied.');
 					}
 				}
 			}
 		);
 
 		$dispatcher->addListener(
-			'file.beforeCreateZip',
-			function (GenericEvent $event) use ($userSession, $rootFolder) {
-				$dir = $event->getArgument('dir');
-				$files = $event->getArgument('files');
+			BeforeZipCreatedEvent::class,
+			function (BeforeZipCreatedEvent $event) use ($userSession, $rootFolder): void {
+				$dir = $event->getDirectory();
+				$files = $event->getFiles();
 
 				$pathsToCheck = [];
-				if (\is_array($files)) {
-					foreach ($files as $file) {
-						$pathsToCheck[] = $dir . '/' . $file;
-					}
-				} elseif (\is_string($files)) {
-					$pathsToCheck[] = $dir . '/' . $files;
+				foreach ($files as $file) {
+					$pathsToCheck[] = $dir . '/' . $file;
 				}
 
 				// Check only for user/group shares. Don't restrict e.g. share links
-				if ($userSession && $userSession->isLoggedIn()) {
-					$uid = $userSession->getUser()->getUID();
+				$user = $userSession->getUser();
+				if ($user) {
 					$viewOnlyHandler = new ViewOnly(
-						$rootFolder->getUserFolder($uid)
+						$rootFolder->getUserFolder($user->getUID())
 					);
 					if (!$viewOnlyHandler->check($pathsToCheck)) {
-						$event->setArgument('errorMessage', 'Access to this resource or one of its sub-items has been denied.');
-						$event->setArgument('run', false);
+						$event->setErrorMessage('Access to this resource or one of its sub-items has been denied.');
+						$event->setSuccessful(false);
 					} else {
-						$event->setArgument('run', true);
+						$event->setSuccessful(true);
 					}
 				} else {
-					$event->setArgument('run', true);
+					$event->setSuccessful(true);
 				}
 			}
 		);

apps/files_sharing/lib/Controller/PublicPreviewController.php

@@ -136,7 +136,7 @@ class PublicPreviewController extends PublicShareController {
 	 * @param $token
 	 * @return DataResponse|FileDisplayResponse
 	 */
-	public function directLink($token) {
+	public function directLink(string $token) {
 		// No token no image
 		if ($token === '') {
 			return new DataResponse([], Http::STATUS_BAD_REQUEST);

apps/files_sharing/lib/Controller/ShareAPIController.php

@@ -45,6 +45,7 @@ declare(strict_types=1);
 namespace OCA\Files_Sharing\Controller;
 
 use OC\Files\FileInfo;
+use OC\Files\Storage\Wrapper\Wrapper;
 use OCA\Files_Sharing\Exceptions\SharingRightsException;
 use OCA\Files_Sharing\External\Storage;
 use OCA\Files_Sharing\SharedStorage;
@@ -524,14 +525,7 @@ class ShareAPIController extends OCSController {
 			$permissions &= ~($permissions & ~$node->getPermissions());
 		}
 
-		if ($share->getNode()->getStorage()->instanceOfStorage(SharedStorage::class)) {
-			/** @var \OCA\Files_Sharing\SharedStorage $storage */
-			$inheritedAttributes = $share->getNode()->getStorage()->getShare()->getAttributes();
-			if ($inheritedAttributes !== null && $inheritedAttributes->getAttribute('permissions', 'download') === false) {
-				$share->setHideDownload(true);
-			}
-		}
-
+		$this->checkInheritedAttributes($share);
 
 		if ($shareType === IShare::TYPE_USER) {
 			// Valid user is required to share
@@ -1110,6 +1104,25 @@ class ShareAPIController extends OCSController {
 			$share->setNote($note);
 		}
 
+		$userFolder = $this->rootFolder->getUserFolder($this->currentUser);
+
+		// get the node with the point of view of the current user
+		$nodes = $userFolder->getById($share->getNode()->getId());
+		if (count($nodes) > 0) {
+			$node = $nodes[0];
+			$storage = $node->getStorage();
+			if ($storage && $storage->instanceOfStorage(SharedStorage::class)) {
+				/** @var \OCA\Files_Sharing\SharedStorage $storage */
+				$inheritedAttributes = $storage->getShare()->getAttributes();
+				if ($inheritedAttributes !== null && $inheritedAttributes->getAttribute('permissions', 'download') === false) {
+					if ($hideDownload === 'false') {
+						throw new OCSBadRequestException($this->l->t('Cannot increase permissions'));
+					}
+					$share->setHideDownload(true);
+				}
+			}
+		}
+
 		/**
 		 * expirationdate, password and publicUpload only make sense for link shares
 		 */
@@ -1135,24 +1148,6 @@ class ShareAPIController extends OCSController {
 				$share->setHideDownload(false);
 			}
 
-			$userFolder = $this->rootFolder->getUserFolder($this->currentUser);
-			// get the node with the point of view of the current user
-			$nodes = $userFolder->getById($share->getNode()->getId());
-			if (count($nodes) > 0) {
-				$node = $nodes[0];
-				$storage = $node->getStorage();
-				if ($storage->instanceOfStorage(SharedStorage::class)) {
-					/** @var \OCA\Files_Sharing\SharedStorage $storage */
-					$inheritedAttributes = $storage->getShare()->getAttributes();
-					if ($inheritedAttributes !== null && $inheritedAttributes->getAttribute('permissions', 'download') === false) {
-						if ($hideDownload === 'false') {
-							throw new OCSBadRequestException($this->l->t('Cannot increate permissions'));
-						}
-						$share->setHideDownload(true);
-					}
-				}
-			}
-
 			$newPermissions = null;
 			if ($publicUpload === 'true') {
 				$newPermissions = Constants::PERMISSION_READ | Constants::PERMISSION_CREATE | Constants::PERMISSION_UPDATE | Constants::PERMISSION_DELETE;
@@ -1905,4 +1900,24 @@ class ShareAPIController extends OCSController {
 
 		return $share;
 	}
+
+	private function checkInheritedAttributes(IShare $share): void {
+		if ($share->getNode()->getStorage()->instanceOfStorage(SharedStorage::class)) {
+			$storage = $share->getNode()->getStorage();
+			if ($storage instanceof Wrapper) {
+				$storage = $storage->getInstanceOfStorage(SharedStorage::class);
+				if ($storage === null) {
+					throw new \RuntimeException('Should not happen, instanceOfStorage but getInstanceOfStorage return null');
+				}
+			} else {
+				throw new \RuntimeException('Should not happen, instanceOfStorage but not a wrapper');
+			}
+			/** @var \OCA\Files_Sharing\SharedStorage $storage */
+			$inheritedAttributes = $storage->getShare()->getAttributes();
+			if ($inheritedAttributes !== null && $inheritedAttributes->getAttribute('permissions', 'download') === false) {
+				$share->setHideDownload(true);
+			}
+		}
+
+	}
 }

apps/files_sharing/lib/MountProvider.php

@@ -241,8 +241,9 @@ class MountProvider implements IMountProvider {
 				$superPermissions |= $share->getPermissions();
 
 				// update share permission attributes
-				if ($share->getAttributes() !== null) {
-					foreach ($share->getAttributes()->toArray() as $attribute) {
+				$attributes = $share->getAttributes();
+				if ($attributes !== null) {
+					foreach ($attributes->toArray() as $attribute) {
 						if ($superAttributes->getAttribute($attribute['scope'], $attribute['key']) === true) {
 							// if super share attribute is already enabled, it is most permissive
 							continue;

apps/files_sharing/src/components/SharingEntry.vue

@@ -80,8 +80,9 @@
 
 				<ActionCheckbox ref="canDownload"
 					:checked.sync="canDownload"
+					v-if="isSetDownloadButtonVisible"
 					:disabled="saving || !canSetDownload">
-					{{ t('files_sharing', 'Allow download') }}
+					{{ allowDownloadText }}
 				</ActionCheckbox>
 
 				<!-- expiration date -->
@@ -407,6 +408,36 @@ export default {
 			return (typeof this.share.status === 'object' && !Array.isArray(this.share.status))
 		},
 
+		/**
+		 * @return {string}
+		 */
+		allowDownloadText() {
+			if (this.isFolder) {
+				return t('files_sharing', 'Allow download of office files')
+			} else {
+				return t('files_sharing', 'Allow download')
+			}
+		},
+
+		/**
+		 * @return {boolean}
+		 */
+		isSetDownloadButtonVisible() {
+			const allowedMimetypes = [
+				// Office documents
+				'application/msword',
+				'application/vnd.openxmlformats-officedocument.wordprocessingml.document',
+				'application/vnd.ms-powerpoint',
+				'application/vnd.openxmlformats-officedocument.presentationml.presentation',
+				'application/vnd.ms-excel',
+				'application/vnd.openxmlformats-officedocument.spreadsheetml.sheet',
+				'application/vnd.oasis.opendocument.text',
+				'application/vnd.oasis.opendocument.spreadsheet',
+				'application/vnd.oasis.opendocument.presentation',
+			]
+
+			return this.isFolder || allowedMimetypes.includes(this.fileInfo.mimetype)
+		},
 	},
 
 	methods: {

apps/files_sharing/src/components/SharingEntryLink.vue

@@ -158,7 +158,7 @@
 					<ActionSeparator />
 
 					<ActionCheckbox :checked.sync="share.hideDownload"
-						:disabled="saving"
+						:disabled="saving || canChangeHideDownload"
 						@change="queueUpdate('hideDownload')">
 						{{ t('files_sharing', 'Hide download') }}
 					</ActionCheckbox>
@@ -606,6 +606,12 @@ export default {
 		isPasswordPolicyEnabled() {
 			return typeof this.config.passwordPolicy === 'object'
 		},
+
+		canChangeHideDownload() {
+			const hasDisabledDownload = (shareAttribute) => shareAttribute.key === 'download' && shareAttribute.scope === 'permissions' && shareAttribute.enabled === false
+
+			return this.fileInfo.shareAttributes.some(hasDisabledDownload)
+		},
 	},
 
 	methods: {
@@ -867,7 +873,6 @@ export default {
 			this.$emit('remove:share', this.share)
 		},
 	},
-
 }
 </script>
 

apps/files_sharing/tests/ApplicationTest.php

@@ -22,6 +22,8 @@
  */
 namespace OCA\Files_Sharing\Tests;
 
+use OCP\Files\Events\BeforeDirectFileDownloadEvent;
+use OCP\Files\Events\BeforeZipCreatedEvent;
 use Psr\Log\LoggerInterface;
 use OC\Share20\LegacyHooks;
 use OC\Share20\Manager;
@@ -32,6 +34,7 @@ use OCP\Constants;
 use OCP\EventDispatcher\GenericEvent;
 use OCP\EventDispatcher\IEventDispatcher;
 use OCP\Files\Cache\ICacheEntry;
+use OCP\Files\Event\BeforeDirectGetEvent;
 use OCP\Files\File;
 use OCP\Files\Folder;
 use OCP\Files\IRootFolder;
@@ -45,12 +48,8 @@ use Symfony\Component\EventDispatcher\EventDispatcher as SymfonyDispatcher;
 use Test\TestCase;
 
 class ApplicationTest extends TestCase {
-
-	/** @var Application */
-	private $application;
-
-	/** @var IEventDispatcher */
-	private $eventDispatcher;
+	private Application $application;
+	private IEventDispatcher $eventDispatcher;
 
 	/** @var IUserSession */
 	private $userSession;
@@ -81,7 +80,7 @@ class ApplicationTest extends TestCase {
 		);
 	}
 
-	public function providesDataForCanGet() {
+	public function providesDataForCanGet(): array {
 		// normal file (sender) - can download directly
 		$senderFileStorage = $this->createMock(IStorage::class);
 		$senderFileStorage->method('instanceOfStorage')->with(SharedStorage::class)->willReturn(false);
@@ -130,7 +129,7 @@ class ApplicationTest extends TestCase {
 	/**
 	 * @dataProvider providesDataForCanGet
 	 */
-	public function testCheckDirectCanBeDownloaded($path, $userFolder, $run) {
+	public function testCheckDirectCanBeDownloaded(string $path, Folder $userFolder, bool $run): void {
 		$user = $this->createMock(IUser::class);
 		$user->method('getUID')->willReturn('test');
 		$this->userSession->method('getUser')->willReturn($user);
@@ -138,13 +137,13 @@ class ApplicationTest extends TestCase {
 		$this->rootFolder->method('getUserFolder')->willReturn($userFolder);
 
 		// Simulate direct download of file
-		$event = new GenericEvent(null, [ 'path' => $path ]);
-		$this->eventDispatcher->dispatch('file.beforeGetDirect', $event);
+		$event = new BeforeDirectFileDownloadEvent($path);
+		$this->eventDispatcher->dispatchTyped($event);
 
-		$this->assertEquals($run, !$event->hasArgument('errorMessage'));
+		$this->assertEquals($run, $event->isSuccessful());
 	}
 
-	public function providesDataForCanZip() {
+	public function providesDataForCanZip(): array {
 		// Mock: Normal file/folder storage
 		$nonSharedStorage = $this->createMock(IStorage::class);
 		$nonSharedStorage->method('instanceOfStorage')->with(SharedStorage::class)->willReturn(false);
@@ -172,7 +171,7 @@ class ApplicationTest extends TestCase {
 		$sender1UserFolder->method('get')->willReturn($sender1RootFolder);
 
 		$return[] = [ '/folder', ['bar1.txt', 'bar2.txt'], $sender1UserFolder, true ];
-		$return[] = [ '/', 'folder', $sender1UserFolder, true ];
+		$return[] = [ '/', ['folder'], $sender1UserFolder, true ];
 
 		// 3. cannot download zipped 1 non-shared file and 1 secure-shared inside non-shared folder
 		$receiver1File = $this->createMock(File::class);
@@ -199,7 +198,7 @@ class ApplicationTest extends TestCase {
 		$receiver2UserFolder = $this->createMock(Folder::class);
 		$receiver2UserFolder->method('get')->willReturn($receiver2RootFolder);
 
-		$return[] = [ '/', 'secured-folder', $receiver2UserFolder, false ];
+		$return[] = [ '/', ['secured-folder'], $receiver2UserFolder, false ];
 
 		return $return;
 	}
@@ -207,7 +206,7 @@ class ApplicationTest extends TestCase {
 	/**
 	 * @dataProvider providesDataForCanZip
 	 */
-	public function testCheckZipCanBeDownloaded($dir, $files, $userFolder, $run) {
+	public function testCheckZipCanBeDownloaded(string $dir, array $files, Folder $userFolder, bool $run): void {
 		$user = $this->createMock(IUser::class);
 		$user->method('getUID')->willReturn('test');
 		$this->userSession->method('getUser')->willReturn($user);
@@ -216,22 +215,22 @@ class ApplicationTest extends TestCase {
 		$this->rootFolder->method('getUserFolder')->with('test')->willReturn($userFolder);
 
 		// Simulate zip download of folder folder
-		$event = new GenericEvent(null, ['dir' => $dir, 'files' => $files, 'run' => true]);
-		$this->eventDispatcher->dispatch('file.beforeCreateZip', $event);
+		$event = new BeforeZipCreatedEvent($dir, $files);
+		$this->eventDispatcher->dispatchTyped($event);
 
-		$this->assertEquals($run, $event->getArgument('run'));
-		$this->assertEquals($run, !$event->hasArgument('errorMessage'));
+		$this->assertEquals($run, $event->isSuccessful());
+		$this->assertEquals($run, $event->getErrorMessage() === null);
 	}
 
-	public function testCheckFileUserNotFound() {
+	public function testCheckFileUserNotFound(): void {
 		$this->userSession->method('isLoggedIn')->willReturn(false);
 
 		// Simulate zip download of folder folder
-		$event = new GenericEvent(null, ['dir' => '/test', 'files' => ['test.txt'], 'run' => true]);
-		$this->eventDispatcher->dispatch('file.beforeCreateZip', $event);
+		$event = new BeforeZipCreatedEvent('/test', ['test.txt']);
+		$this->eventDispatcher->dispatchTyped($event);
 
 		// It should run as this would restrict e.g. share links otherwise
-		$this->assertTrue($event->getArgument('run'));
-		$this->assertFalse($event->hasArgument('errorMessage'));
+		$this->assertTrue($event->isSuccessful());
+		$this->assertEquals(null, $event->getErrorMessage());
 	}
 }

apps/files_sharing/tests/Controller/ShareAPIControllerTest.php

@@ -46,6 +46,7 @@ use OCP\Files\IRootFolder;
 use OCP\Files\Mount\IMountPoint;
 use OCP\Files\NotFoundException;
 use OCP\Files\Storage;
+use OCP\Files\Storage\IStorage;
 use OCP\IConfig;
 use OCP\IGroup;
 use OCP\IGroupManager;
@@ -1675,8 +1676,10 @@ class ShareAPIControllerTest extends TestCase {
 		$path = $this->getMockBuilder(File::class)->getMock();
 		$storage = $this->createMock(Storage::class);
 		$storage->method('instanceOfStorage')
-			->with('OCA\Files_Sharing\External\Storage')
-			->willReturn(false);
+			->willReturnMap([
+				['OCA\Files_Sharing\External\Storage', false],
+				['OCA\Files_Sharing\SharedStorage', false],
+			]);
 		$path->method('getStorage')->willReturn($storage);
 		$userFolder->expects($this->once())
 			->method('get')
@@ -1709,8 +1712,10 @@ class ShareAPIControllerTest extends TestCase {
 		$path = $this->getMockBuilder(File::class)->getMock();
 		$storage = $this->createMock(Storage::class);
 		$storage->method('instanceOfStorage')
-			->with('OCA\Files_Sharing\External\Storage')
-			->willReturn(false);
+			->willReturnMap([
+				['OCA\Files_Sharing\External\Storage', false],
+				['OCA\Files_Sharing\SharedStorage', false],
+			]);
 		$path->method('getStorage')->willReturn($storage);
 		$userFolder->expects($this->once())
 			->method('get')
@@ -1761,8 +1766,10 @@ class ShareAPIControllerTest extends TestCase {
 		$path = $this->getMockBuilder(File::class)->getMock();
 		$storage = $this->createMock(Storage::class);
 		$storage->method('instanceOfStorage')
-			->with('OCA\Files_Sharing\External\Storage')
-			->willReturn(false);
+			->willReturnMap([
+				['OCA\Files_Sharing\External\Storage', false],
+				['OCA\Files_Sharing\SharedStorage', false],
+			]);
 		$path->method('getStorage')->willReturn($storage);
 		$userFolder->expects($this->once())
 				->method('get')
@@ -1817,8 +1824,10 @@ class ShareAPIControllerTest extends TestCase {
 		$path = $this->getMockBuilder(File::class)->getMock();
 		$storage = $this->createMock(Storage::class);
 		$storage->method('instanceOfStorage')
-			->with('OCA\Files_Sharing\External\Storage')
-			->willReturn(false);
+			->willReturnMap([
+				['OCA\Files_Sharing\External\Storage', false],
+				['OCA\Files_Sharing\SharedStorage', false],
+			]);
 		$path->method('getStorage')->willReturn($storage);
 		$userFolder->expects($this->once())
 				->method('get')
@@ -1876,8 +1885,10 @@ class ShareAPIControllerTest extends TestCase {
 		$path = $this->getMockBuilder(Folder::class)->getMock();
 		$storage = $this->createMock(Storage::class);
 		$storage->method('instanceOfStorage')
-			->with('OCA\Files_Sharing\External\Storage')
-			->willReturn(false);
+			->willReturnMap([
+				['OCA\Files_Sharing\External\Storage', false],
+				['OCA\Files_Sharing\SharedStorage', false],
+			]);
 		$path->method('getStorage')->willReturn($storage);
 		$userFolder->expects($this->once())
 			->method('get')
@@ -1930,8 +1941,10 @@ class ShareAPIControllerTest extends TestCase {
 		$path = $this->getMockBuilder(Folder::class)->getMock();
 		$storage = $this->createMock(Storage::class);
 		$storage->method('instanceOfStorage')
-			->with('OCA\Files_Sharing\External\Storage')
-			->willReturn(false);
+			->willReturnMap([
+				['OCA\Files_Sharing\External\Storage', false],
+				['OCA\Files_Sharing\SharedStorage', false],
+			]);
 		$path->method('getStorage')->willReturn($storage);
 		$userFolder->expects($this->once())
 			->method('get')
@@ -1964,8 +1977,10 @@ class ShareAPIControllerTest extends TestCase {
 		$path = $this->getMockBuilder(Folder::class)->getMock();
 		$storage = $this->createMock(Storage::class);
 		$storage->method('instanceOfStorage')
-			->with('OCA\Files_Sharing\External\Storage')
-			->willReturn(false);
+			->willReturnMap([
+				['OCA\Files_Sharing\External\Storage', false],
+				['OCA\Files_Sharing\SharedStorage', false],
+			]);
 		$path->method('getStorage')->willReturn($storage);
 		$this->rootFolder->method('getUserFolder')->with($this->currentUser)->willReturnSelf();
 		$this->rootFolder->method('get')->with('valid-path')->willReturn($path);
@@ -1985,8 +2000,10 @@ class ShareAPIControllerTest extends TestCase {
 		$path = $this->getMockBuilder(Folder::class)->getMock();
 		$storage = $this->createMock(Storage::class);
 		$storage->method('instanceOfStorage')
-			->with('OCA\Files_Sharing\External\Storage')
-			->willReturn(false);
+			->willReturnMap([
+				['OCA\Files_Sharing\External\Storage', false],
+				['OCA\Files_Sharing\SharedStorage', false],
+			]);
 		$path->method('getStorage')->willReturn($storage);
 		$this->rootFolder->method('getUserFolder')->with($this->currentUser)->willReturnSelf();
 		$this->rootFolder->method('get')->with('valid-path')->willReturn($path);
@@ -2007,8 +2024,10 @@ class ShareAPIControllerTest extends TestCase {
 		$path = $this->getMockBuilder(File::class)->getMock();
 		$storage = $this->createMock(Storage::class);
 		$storage->method('instanceOfStorage')
-			->with('OCA\Files_Sharing\External\Storage')
-			->willReturn(false);
+			->willReturnMap([
+				['OCA\Files_Sharing\External\Storage', false],
+				['OCA\Files_Sharing\SharedStorage', false],
+			]);
 		$path->method('getStorage')->willReturn($storage);
 		$this->rootFolder->method('getUserFolder')->with($this->currentUser)->willReturnSelf();
 		$this->rootFolder->method('get')->with('valid-path')->willReturn($path);
@@ -2028,8 +2047,10 @@ class ShareAPIControllerTest extends TestCase {
 		$path = $this->getMockBuilder(Folder::class)->getMock();
 		$storage = $this->createMock(Storage::class);
 		$storage->method('instanceOfStorage')
-			->with('OCA\Files_Sharing\External\Storage')
-			->willReturn(false);
+			->willReturnMap([
+				['OCA\Files_Sharing\External\Storage', false],
+				['OCA\Files_Sharing\SharedStorage', false],
+			]);
 		$path->method('getStorage')->willReturn($storage);
 		$this->rootFolder->method('getUserFolder')->with($this->currentUser)->willReturnSelf();
 		$this->rootFolder->method('get')->with('valid-path')->willReturn($path);
@@ -2064,8 +2085,10 @@ class ShareAPIControllerTest extends TestCase {
 		$path = $this->getMockBuilder(Folder::class)->getMock();
 		$storage = $this->createMock(Storage::class);
 		$storage->method('instanceOfStorage')
-			->with('OCA\Files_Sharing\External\Storage')
-			->willReturn(false);
+			->willReturnMap([
+				['OCA\Files_Sharing\External\Storage', false],
+				['OCA\Files_Sharing\SharedStorage', false],
+			]);
 		$path->method('getStorage')->willReturn($storage);
 		$this->rootFolder->method('getUserFolder')->with($this->currentUser)->willReturnSelf();
 		$this->rootFolder->method('get')->with('valid-path')->willReturn($path);
@@ -2100,8 +2123,10 @@ class ShareAPIControllerTest extends TestCase {
 		$path = $this->getMockBuilder(Folder::class)->getMock();
 		$storage = $this->createMock(Storage::class);
 		$storage->method('instanceOfStorage')
-			->with('OCA\Files_Sharing\External\Storage')
-			->willReturn(false);
+			->willReturnMap([
+				['OCA\Files_Sharing\External\Storage', false],
+				['OCA\Files_Sharing\SharedStorage', false],
+			]);
 		$path->method('getStorage')->willReturn($storage);
 		$this->rootFolder->method('getUserFolder')->with($this->currentUser)->willReturnSelf();
 		$this->rootFolder->method('get')->with('valid-path')->willReturn($path);
@@ -2143,8 +2168,10 @@ class ShareAPIControllerTest extends TestCase {
 		$path = $this->getMockBuilder(Folder::class)->getMock();
 		$storage = $this->createMock(Storage::class);
 		$storage->method('instanceOfStorage')
-			->with('OCA\Files_Sharing\External\Storage')
-			->willReturn(false);
+			->willReturnMap([
+				['OCA\Files_Sharing\External\Storage', false],
+				['OCA\Files_Sharing\SharedStorage', false],
+			]);
 		$path->method('getStorage')->willReturn($storage);
 		$path->method('getPath')->willReturn('valid-path');
 		$this->rootFolder->method('getUserFolder')->with($this->currentUser)->willReturnSelf();
@@ -2179,8 +2206,10 @@ class ShareAPIControllerTest extends TestCase {
 		$path = $this->getMockBuilder(Folder::class)->getMock();
 		$storage = $this->createMock(Storage::class);
 		$storage->method('instanceOfStorage')
-			->with('OCA\Files_Sharing\External\Storage')
-			->willReturn(false);
+			->willReturnMap([
+				['OCA\Files_Sharing\External\Storage', false],
+				['OCA\Files_Sharing\SharedStorage', false],
+			]);
 		$path->method('getStorage')->willReturn($storage);
 		$this->rootFolder->method('getUserFolder')->with($this->currentUser)->willReturnSelf();
 		$this->rootFolder->method('get')->with('valid-path')->willReturn($path);
@@ -2222,8 +2251,10 @@ class ShareAPIControllerTest extends TestCase {
 		$path = $this->getMockBuilder(Folder::class)->getMock();
 		$storage = $this->createMock(Storage::class);
 		$storage->method('instanceOfStorage')
-			->with('OCA\Files_Sharing\External\Storage')
-			->willReturn(false);
+			->willReturnMap([
+				['OCA\Files_Sharing\External\Storage', false],
+				['OCA\Files_Sharing\SharedStorage', false],
+			]);
 		$path->method('getStorage')->willReturn($storage);
 		$this->rootFolder->method('getUserFolder')->with($this->currentUser)->willReturnSelf();
 		$this->rootFolder->method('get')->with('valid-path')->willReturn($path);
@@ -2270,8 +2301,10 @@ class ShareAPIControllerTest extends TestCase {
 		$path = $this->getMockBuilder(File::class)->getMock();
 		$storage = $this->createMock(Storage::class);
 		$storage->method('instanceOfStorage')
-			->with('OCA\Files_Sharing\External\Storage')
-			->willReturn(false);
+			->willReturnMap([
+				['OCA\Files_Sharing\External\Storage', false],
+				['OCA\Files_Sharing\SharedStorage', false],
+			]);
 		$path->method('getStorage')->willReturn($storage);
 		$userFolder->expects($this->once())
 				->method('get')
@@ -2342,8 +2375,10 @@ class ShareAPIControllerTest extends TestCase {
 		$path = $this->getMockBuilder(File::class)->getMock();
 		$storage = $this->createMock(Storage::class);
 		$storage->method('instanceOfStorage')
-			->with('OCA\Files_Sharing\External\Storage')
-			->willReturn(false);
+			->willReturnMap([
+				['OCA\Files_Sharing\External\Storage', false],
+				['OCA\Files_Sharing\SharedStorage', false],
+			]);
 		$path->method('getStorage')->willReturn($storage);
 		$userFolder->expects($this->once())
 				->method('get')
@@ -2396,8 +2431,10 @@ class ShareAPIControllerTest extends TestCase {
 		$path = $this->getMockBuilder(File::class)->getMock();
 		$storage = $this->createMock(Storage::class);
 		$storage->method('instanceOfStorage')
-			->with('OCA\Files_Sharing\External\Storage')
-			->willReturn(false);
+			->willReturnMap([
+				['OCA\Files_Sharing\External\Storage', false],
+				['OCA\Files_Sharing\SharedStorage', false],
+			]);
 		$path->method('getStorage')->willReturn($storage);
 		$userFolder->expects($this->once())
 				->method('get')
@@ -2480,8 +2517,10 @@ class ShareAPIControllerTest extends TestCase {
 		$path = $this->getMockBuilder(File::class)->getMock();
 		$storage = $this->createMock(Storage::class);
 		$storage->method('instanceOfStorage')
-			->with('OCA\Files_Sharing\External\Storage')
-			->willReturn(false);
+			->willReturnMap([
+				['OCA\Files_Sharing\External\Storage', false],
+				['OCA\Files_Sharing\SharedStorage', false],
+			]);
 		$path->method('getStorage')->willReturn($storage);
 		$path->method('getPath')->willReturn('valid-path');
 		$userFolder->expects($this->once())
@@ -2523,8 +2562,10 @@ class ShareAPIControllerTest extends TestCase {
 		$path = $this->getMockBuilder(File::class)->getMock();
 		$storage = $this->createMock(Storage::class);
 		$storage->method('instanceOfStorage')
-			->with('OCA\Files_Sharing\External\Storage')
-			->willReturn(false);
+			->willReturnMap([
+				['OCA\Files_Sharing\External\Storage', false],
+				['OCA\Files_Sharing\SharedStorage', false],
+			]);
 		$path->method('getStorage')->willReturn($storage);
 		$userFolder->expects($this->once())
 				->method('get')
@@ -2604,8 +2645,10 @@ class ShareAPIControllerTest extends TestCase {
 		$path = $this->getMockBuilder(Folder::class)->getMock();
 		$storage = $this->createMock(Storage::class);
 		$storage->method('instanceOfStorage')
-			->with('OCA\Files_Sharing\External\Storage')
-			->willReturn(true);
+			->willReturnMap([
+				['OCA\Files_Sharing\External\Storage', true],
+				['OCA\Files_Sharing\SharedStorage', false],
+			]);
 		$path->method('getStorage')->willReturn($storage);
 		$path->method('getPermissions')->willReturn(\OCP\Constants::PERMISSION_READ);
 		$userFolder->expects($this->once())
@@ -2964,8 +3007,17 @@ class ShareAPIControllerTest extends TestCase {
 		$this->expectExceptionMessage('Invalid date. Format must be YYYY-MM-DD');
 
 		$ocs = $this->mockFormatShare();
+		$userFolder = $this->createMock(Folder::class);
+		$userFolder->method('getById')
+			->with(42)
+			->willReturn([]);
+		$this->rootFolder->method('getUserFolder')
+			->with($this->currentUser)
+			->willReturn($userFolder);
 
 		$folder = $this->getMockBuilder(Folder::class)->getMock();
+		$folder->method('getId')
+			->willReturn(42);
 
 		$share = \OC::$server->getShareManager()->newShare();
 		$share->setPermissions(\OCP\Constants::PERMISSION_ALL)
@@ -3003,8 +3055,16 @@ class ShareAPIControllerTest extends TestCase {
 		$this->expectExceptionMessage('Public upload disabled by the administrator');
 
 		$ocs = $this->mockFormatShare();
+		$userFolder = $this->createMock(Folder::class);
+		$userFolder->method('getById')
+			->with(42)
+			->willReturn([]);
+		$this->rootFolder->method('getUserFolder')
+			->with($this->currentUser)
+			->willReturn($userFolder);
 
 		$folder = $this->getMockBuilder(Folder::class)->getMock();
+		$folder->method('getId')->willReturn(42);
 
 		$share = \OC::$server->getShareManager()->newShare();
 		$share->setPermissions(\OCP\Constants::PERMISSION_ALL)
@@ -3026,6 +3086,15 @@ class ShareAPIControllerTest extends TestCase {
 		$ocs = $this->mockFormatShare();
 
 		$file = $this->getMockBuilder(File::class)->getMock();
+		$file->method('getId')
+			->willReturn(42);
+		$userFolder = $this->createMock(Folder::class);
+		$userFolder->method('getById')
+			->with(42)
+			->willReturn([]);
+		$this->rootFolder->method('getUserFolder')
+			->with($this->currentUser)
+			->willReturn($userFolder);
 
 		$share = \OC::$server->getShareManager()->newShare();
 		$share->setPermissions(\OCP\Constants::PERMISSION_ALL)
@@ -3039,13 +3108,21 @@ class ShareAPIControllerTest extends TestCase {
 		$ocs->updateShare(42, null, 'password', null, 'true', '');
 	}
 
-	public function testUpdateLinkSharePasswordDoesNotChangeOther() {
+	public function testUpdateLinkSharePasswordDoesNotChangeOther(): void {
 		$ocs = $this->mockFormatShare();
 
 		$date = new \DateTime('2000-01-01');
 		$date->setTime(0,0,0);
 
 		$node = $this->getMockBuilder(File::class)->getMock();
+		$node->method('getId')->willReturn(42);
+		$userFolder = $this->createMock(Folder::class);
+		$userFolder->method('getById')
+			->with(42)
+			->willReturn([]);
+		$this->rootFolder->method('getUserFolder')
+			->with($this->currentUser)
+			->willReturn($userFolder);
 		$share = $this->newShare();
 		$share->setPermissions(\OCP\Constants::PERMISSION_ALL)
 			->setSharedBy($this->currentUser)
@@ -3090,7 +3167,15 @@ class ShareAPIControllerTest extends TestCase {
 		$date = new \DateTime('2000-01-01');
 		$date->setTime(0,0,0);
 
+		$userFolder = $this->createMock(Folder::class);
+		$userFolder->method('getById')
+			->with(42)
+			->willReturn([]);
+		$this->rootFolder->method('getUserFolder')
+			->with($this->currentUser)
+			->willReturn($userFolder);
 		$node = $this->getMockBuilder(File::class)->getMock();
+		$node->method('getId')->willReturn(42);
 		$share = $this->newShare();
 		$share->setPermissions(\OCP\Constants::PERMISSION_ALL)
 			->setSharedBy($this->currentUser)
@@ -3141,7 +3226,15 @@ class ShareAPIControllerTest extends TestCase {
 		$date = new \DateTime('2000-01-01');
 		$date->setTime(0,0,0);
 
+		$userFolder = $this->createMock(Folder::class);
+		$userFolder->method('getById')
+			->with(42)
+			->willReturn([]);
+		$this->rootFolder->method('getUserFolder')
+			->with($this->currentUser)
+			->willReturn($userFolder);
 		$node = $this->getMockBuilder(File::class)->getMock();
+		$node->method('getId')->willReturn(42);
 		$share = $this->newShare();
 		$share->setPermissions(\OCP\Constants::PERMISSION_ALL)
 			->setSharedBy($this->currentUser)
@@ -3174,7 +3267,15 @@ class ShareAPIControllerTest extends TestCase {
 		$date = new \DateTime('2000-01-01');
 		$date->setTime(0,0,0);
 
+		$userFolder = $this->createMock(Folder::class);
+		$userFolder->method('getById')
+			->with(42)
+			->willReturn([]);
+		$this->rootFolder->method('getUserFolder')
+			->with($this->currentUser)
+			->willReturn($userFolder);
 		$node = $this->getMockBuilder(File::class)->getMock();
+		$node->method('getId')->willReturn(42);
 		$share = $this->newShare();
 		$share->setPermissions(\OCP\Constants::PERMISSION_ALL)
 			->setSharedBy($this->currentUser)

lib/composer/composer/autoload_classmap.php

@@ -258,8 +258,10 @@ return array(
     'OCP\\Files\\Config\\IUserMountCache' => $baseDir . '/lib/public/Files/Config/IUserMountCache.php',
     'OCP\\Files\\EmptyFileNameException' => $baseDir . '/lib/public/Files/EmptyFileNameException.php',
     'OCP\\Files\\EntityTooLargeException' => $baseDir . '/lib/public/Files/EntityTooLargeException.php',
+    'OCP\\Files\\Events\\BeforeDirectFileDownloadEvent' => $baseDir . '/lib/public/Files/Events/BeforeDirectFileDownloadEvent.php',
     'OCP\\Files\\Events\\BeforeFileScannedEvent' => $baseDir . '/lib/public/Files/Events/BeforeFileScannedEvent.php',
     'OCP\\Files\\Events\\BeforeFolderScannedEvent' => $baseDir . '/lib/public/Files/Events/BeforeFolderScannedEvent.php',
+    'OCP\\Files\\Events\\BeforeZipCreatedEvent' => $baseDir . '/lib/public/Files/Events/BeforeZipCreatedEvent.php',
     'OCP\\Files\\Events\\FileCacheUpdated' => $baseDir . '/lib/public/Files/Events/FileCacheUpdated.php',
     'OCP\\Files\\Events\\FileScannedEvent' => $baseDir . '/lib/public/Files/Events/FileScannedEvent.php',
     'OCP\\Files\\Events\\FolderScannedEvent' => $baseDir . '/lib/public/Files/Events/FolderScannedEvent.php',

lib/composer/composer/autoload_static.php

@@ -287,8 +287,10 @@ class ComposerStaticInit53792487c5a8370acc0b06b1a864ff4c
         'OCP\\Files\\Config\\IUserMountCache' => __DIR__ . '/../../..' . '/lib/public/Files/Config/IUserMountCache.php',
         'OCP\\Files\\EmptyFileNameException' => __DIR__ . '/../../..' . '/lib/public/Files/EmptyFileNameException.php',
         'OCP\\Files\\EntityTooLargeException' => __DIR__ . '/../../..' . '/lib/public/Files/EntityTooLargeException.php',
+        'OCP\\Files\\Events\\BeforeDirectFileDownloadEvent' => __DIR__ . '/../../..' . '/lib/public/Files/Events/BeforeDirectFileDownloadEvent.php',
         'OCP\\Files\\Events\\BeforeFileScannedEvent' => __DIR__ . '/../../..' . '/lib/public/Files/Events/BeforeFileScannedEvent.php',
         'OCP\\Files\\Events\\BeforeFolderScannedEvent' => __DIR__ . '/../../..' . '/lib/public/Files/Events/BeforeFolderScannedEvent.php',
+        'OCP\\Files\\Events\\BeforeZipCreatedEvent' => __DIR__ . '/../../..' . '/lib/public/Files/Events/BeforeZipCreatedEvent.php',
         'OCP\\Files\\Events\\FileCacheUpdated' => __DIR__ . '/../../..' . '/lib/public/Files/Events/FileCacheUpdated.php',
         'OCP\\Files\\Events\\FileScannedEvent' => __DIR__ . '/../../..' . '/lib/public/Files/Events/FileScannedEvent.php',
         'OCP\\Files\\Events\\FolderScannedEvent' => __DIR__ . '/../../..' . '/lib/public/Files/Events/FolderScannedEvent.php',

lib/private/Share20/DefaultShareProvider.php

@@ -1543,14 +1543,15 @@ class DefaultShareProvider implements IShareProvider {
 	/**
 	 * Load from database format (JSON string) to IAttributes
 	 *
-	 * @param IShare $share
-	 * @param string|null $data
-	 * @return IShare modified share
+	 * @return IShare the modified share
 	 */
-	private function updateShareAttributes(IShare $share, ?string $data) {
+	private function updateShareAttributes(IShare $share, ?string $data): IShare {
 		if ($data !== null && $data !== '') {
 			$attributes = new ShareAttributes();
 			$compressedAttributes = \json_decode($data, true);
+			if ($compressedAttributes === false || $compressedAttributes === null) {
+				return $share;
+			}
 			foreach ($compressedAttributes as $compressedAttribute) {
 				$attributes->setAttribute(
 					$compressedAttribute[0],
@@ -1566,11 +1567,8 @@ class DefaultShareProvider implements IShareProvider {
 
 	/**
 	 * Format IAttributes to database format (JSON string)
-	 *
-	 * @param IAttributes|null $attributes
-	 * @return string|null
 	 */
-	private function formatShareAttributes($attributes) {
+	private function formatShareAttributes(?IAttributes $attributes): ?string {
 		if ($attributes === null || empty($attributes->toArray())) {
 			return null;
 		}

lib/private/legacy/OC_Files.php

@@ -44,12 +44,12 @@ use bantu\IniGetWrapper\IniGetWrapper;
 use OC\Files\View;
 use OC\Streamer;
 use OCP\Lock\ILockingProvider;
-use OCP\EventDispatcher\GenericEvent;
+use OCP\Files\Events\BeforeZipCreatedEvent;
+use OCP\Files\Events\BeforeDirectFileDownloadEvent;
 use OCP\EventDispatcher\IEventDispatcher;
 
 /**
  * Class for file server access
- *
  */
 class OC_Files {
 	public const FILE = 1;
@@ -165,11 +165,11 @@ class OC_Files {
 			}
 
 			//Dispatch an event to see if any apps have problem with download
-			$event = new GenericEvent(null, ['dir' => $dir, 'files' => $files, 'run' => true]);
-			$dispatcher = \OC::$server->query(IEventDispatcher::class);
-			$dispatcher->dispatch('file.beforeCreateZip', $event);
-			if (($event->getArgument('run') === false) or ($event->hasArgument('errorMessage'))) {
-				throw new \OC\ForbiddenException($event->getArgument('errorMessage'));
+			$event = new BeforeZipCreatedEvent($dir, is_array($files) ? $files : [$files]);
+			$dispatcher = \OC::$server->get(IEventDispatcher::class);
+			$dispatcher->dispatchTyped($event);
+			if ((!$event->isSuccessful()) || $event->getErrorMessage() !== null) {
+				throw new \OC\ForbiddenException($event->getErrorMessage());
 			}
 
 			$streamer = new Streamer(\OC::$server->getRequest(), $fileSize, $numberOfFiles);
@@ -217,8 +217,6 @@ class OC_Files {
 			$streamer->finalize();
 			set_time_limit($executionTime);
 			self::unlockAllTheFiles($dir, $files, $getType, $view, $filename);
-			$event = new GenericEvent(null, ['result' => 'success', 'dir' => $dir, 'files' => $files]);
-			$dispatcher->dispatch('file.afterCreateZip', $event);
 		} catch (\OCP\Lock\LockedException $ex) {
 			self::unlockAllTheFiles($dir, $files, $getType, $view, $filename);
 			OC::$server->getLogger()->logException($ex);
@@ -235,8 +233,8 @@ class OC_Files {
 			OC::$server->getLogger()->logException($ex);
 			$l = \OC::$server->getL10N('lib');
 			$hint = method_exists($ex, 'getHint') ? $ex->getHint() : '';
-			if ($event && $event->hasArgument('message')) {
-				$hint .= ' ' . $event->getArgument('message');
+			if ($event && $event->getErrorMessage() !== null) {
+				$hint .= ' ' . $event->getErrorMessage();
 			}
 			\OC_Template::printErrorPage($l->t('Cannot download file'), $hint, 200);
 		}
@@ -334,12 +332,12 @@ class OC_Files {
 		}
 
 		$dispatcher = \OC::$server->query(IEventDispatcher::class);
-		$event = new GenericEvent(null, ['path' => $filename]);
-		$dispatcher->dispatch('file.beforeGetDirect', $event);
+		$event = new BeforeDirectFileDownloadEvent($filename);
+		$dispatcher->dispatchTyped($event);
 
-		if (!\OC\Files\Filesystem::isReadable($filename) || $event->hasArgument('errorMessage')) {
-			if (!$event->hasArgument('errorMessage')) {
-				$msg = $event->getArgument('errorMessage');
+		if (!\OC\Files\Filesystem::isReadable($filename) || $event->getErrorMessage()) {
+			if ($event->getErrorMessage()) {
+				$msg = $event->getErrorMessage();
 			} else {
 				$msg = 'Access denied';
 			}

lib/public/Files/Events/BeforeDirectFileDownloadEvent.php

@@ -0,0 +1,84 @@
+<?php
+
+declare(strict_types=1);
+
+/**
+ * @copyright 2022 Carl Schwan <carl@carlschwan.eu>
+ *
+ * @license GNU AGPL version 3 or any later version
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as
+ * published by the Free Software Foundation, either version 3 of the
+ * License, or (at your option) any later version.
+ *
+ * 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
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+namespace OCP\Files\Events;
+
+use OCP\EventDispatcher\Event;
+
+/**
+ * This event is triggered when a user tries to download a file
+ * directly.
+ *
+ * @since 25.0.0
+ */
+class BeforeDirectFileDownloadEvent extends Event {
+	private string $path;
+	private bool $successful = true;
+	private ?string $errorMessage = null;
+
+	/**
+	 * @since 25.0.0
+	 */
+	public function __construct(string $path) {
+		parent::__construct();
+		$this->path = $path;
+	}
+
+	/**
+	 * @since 25.0.0
+	 */
+	public function getPath(): string {
+		return $this->path;
+	}
+
+	/**
+	 * @since 25.0.0
+	 */
+	public function isSuccessful(): bool {
+		return $this->successful;
+	}
+
+	/**
+	 * Set if the event was successful
+	 *
+	 * @since 25.0.0
+	 */
+	public function setSuccessful(bool $successful): void {
+		$this->successful = $successful;
+	}
+
+	/**
+	 * Get the error message, if any
+	 * @since 25.0.0
+	 */
+	public function getErrorMessage(): ?string {
+		return $this->errorMessage;
+	}
+
+	/**
+	 * @since 25.0.0
+	 */
+	public function setErrorMessage(string $errorMessage): void {
+		$this->errorMessage = $errorMessage;
+	}
+}

lib/public/Files/Events/BeforeZipCreatedEvent.php

@@ -0,0 +1,91 @@
+<?php
+
+declare(strict_types=1);
+
+/**
+ * @copyright 2022 Carl Schwan <carl@carlschwan.eu>
+ *
+ * @license GNU AGPL version 3 or any later version
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as
+ * published by the Free Software Foundation, either version 3 of the
+ * License, or (at your option) any later version.
+ *
+ * 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
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+namespace OCP\Files\Events;
+
+use OCP\EventDispatcher\Event;
+
+/**
+ * @since 25.0.0
+ */
+class BeforeZipCreatedEvent extends Event {
+	private string $directory;
+	private array $files;
+	private bool $successful = true;
+	private ?string $errorMessage = null;
+
+	/**
+	 * @since 25.0.0
+	 */
+	public function __construct(string $directory, array $files) {
+		parent::__construct();
+		$this->directory = $directory;
+		$this->files = $files;
+	}
+
+	/**
+	 * @since 25.0.0
+	 */
+	public function getDirectory(): string {
+		return $this->directory;
+	}
+
+	/**
+	 * @since 25.0.0
+	 */
+	public function getFiles(): array {
+		return $this->files;
+	}
+
+	/**
+	 * @since 25.0.0
+	 */
+	public function isSuccessful(): bool {
+		return $this->successful;
+	}
+
+	/**
+	 * Set if the event was successful
+	 *
+	 * @since 25.0.0
+	 */
+	public function setSuccessful(bool $successful): void {
+		$this->successful = $successful;
+	}
+
+	/**
+	 * Get the error message, if any
+	 * @since 25.0.0
+	 */
+	public function getErrorMessage(): ?string {
+		return $this->errorMessage;
+	}
+
+	/**
+	 * @since 25.0.0
+	 */
+	public function setErrorMessage(string $errorMessage): void {
+		$this->errorMessage = $errorMessage;
+	}
+}