Merge pull request #51458 from nextcloud/fix/fix-public-download-activity

Fix public download activity

.github/workflows/integration-sqlite.yml

@@ -73,6 +73,7 @@ jobs:
 
         php-versions: ['8.1']
         spreed-versions: ['main']
+        activity-versions: ['master']
 
     services:
       redis:
@@ -104,6 +105,14 @@ jobs:
           path: apps/spreed
           ref: ${{ matrix.spreed-versions }}
 
+      - name: Checkout Activity app
+        if: ${{ matrix.test-suite == 'sharing_features' }}
+        uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2
+        with:
+          repository: nextcloud/activity
+          path: apps/activity
+          ref: ${{ matrix.activity-versions }}
+
       - name: Set up php ${{ matrix.php-versions }}
         uses: shivammathur/setup-php@9e72090525849c5e82e596468b86eb55e9cc5401 #v2.32.0
         with:

apps/files_sharing/composer/composer/autoload_classmap.php

@@ -59,6 +59,7 @@ return array(
     'OCA\\Files_Sharing\\ISharedMountPoint' => $baseDir . '/../lib/ISharedMountPoint.php',
     'OCA\\Files_Sharing\\ISharedStorage' => $baseDir . '/../lib/ISharedStorage.php',
     'OCA\\Files_Sharing\\Listener\\BeforeDirectFileDownloadListener' => $baseDir . '/../lib/Listener/BeforeDirectFileDownloadListener.php',
+    'OCA\\Files_Sharing\\Listener\\BeforeNodeReadListener' => $baseDir . '/../lib/Listener/BeforeNodeReadListener.php',
     'OCA\\Files_Sharing\\Listener\\BeforeZipCreatedListener' => $baseDir . '/../lib/Listener/BeforeZipCreatedListener.php',
     'OCA\\Files_Sharing\\Listener\\LoadAdditionalListener' => $baseDir . '/../lib/Listener/LoadAdditionalListener.php',
     'OCA\\Files_Sharing\\Listener\\LoadPublicFileRequestAuthListener' => $baseDir . '/../lib/Listener/LoadPublicFileRequestAuthListener.php',

apps/files_sharing/composer/composer/autoload_static.php

@@ -74,6 +74,7 @@ class ComposerStaticInitFiles_Sharing
         'OCA\\Files_Sharing\\ISharedMountPoint' => __DIR__ . '/..' . '/../lib/ISharedMountPoint.php',
         'OCA\\Files_Sharing\\ISharedStorage' => __DIR__ . '/..' . '/../lib/ISharedStorage.php',
         'OCA\\Files_Sharing\\Listener\\BeforeDirectFileDownloadListener' => __DIR__ . '/..' . '/../lib/Listener/BeforeDirectFileDownloadListener.php',
+        'OCA\\Files_Sharing\\Listener\\BeforeNodeReadListener' => __DIR__ . '/..' . '/../lib/Listener/BeforeNodeReadListener.php',
         'OCA\\Files_Sharing\\Listener\\BeforeZipCreatedListener' => __DIR__ . '/..' . '/../lib/Listener/BeforeZipCreatedListener.php',
         'OCA\\Files_Sharing\\Listener\\LoadAdditionalListener' => __DIR__ . '/..' . '/../lib/Listener/LoadAdditionalListener.php',
         'OCA\\Files_Sharing\\Listener\\LoadPublicFileRequestAuthListener' => __DIR__ . '/..' . '/../lib/Listener/LoadPublicFileRequestAuthListener.php',

apps/files_sharing/lib/AppInfo/Application.php

@@ -16,6 +16,7 @@ use OCA\Files_Sharing\External\Manager;
 use OCA\Files_Sharing\External\MountProvider as ExternalMountProvider;
 use OCA\Files_Sharing\Helper;
 use OCA\Files_Sharing\Listener\BeforeDirectFileDownloadListener;
+use OCA\Files_Sharing\Listener\BeforeNodeReadListener;
 use OCA\Files_Sharing\Listener\BeforeZipCreatedListener;
 use OCA\Files_Sharing\Listener\LoadAdditionalListener;
 use OCA\Files_Sharing\Listener\LoadPublicFileRequestAuthListener;
@@ -42,6 +43,7 @@ use OCP\Federation\ICloudIdManager;
 use OCP\Files\Config\IMountProviderCollection;
 use OCP\Files\Events\BeforeDirectFileDownloadEvent;
 use OCP\Files\Events\BeforeZipCreatedEvent;
+use OCP\Files\Events\Node\BeforeNodeReadEvent;
 use OCP\Group\Events\GroupChangedEvent;
 use OCP\Group\Events\GroupDeletedEvent;
 use OCP\Group\Events\UserAddedEvent;
@@ -94,9 +96,13 @@ class Application extends App implements IBootstrap {
 		$context->registerEventListener(ShareCreatedEvent::class, UserShareAcceptanceListener::class);
 		$context->registerEventListener(UserAddedEvent::class, UserAddedToGroupListener::class);
 
-		// Handle download events for view only checks
-		$context->registerEventListener(BeforeZipCreatedEvent::class, BeforeZipCreatedListener::class);
-		$context->registerEventListener(BeforeDirectFileDownloadEvent::class, BeforeDirectFileDownloadListener::class);
+		// Publish activity for public download
+		$context->registerEventListener(BeforeNodeReadEvent::class, BeforeNodeReadListener::class);
+		$context->registerEventListener(BeforeZipCreatedEvent::class, BeforeNodeReadListener::class);
+
+		// Handle download events for view only checks. Priority higher than 0 to run early.
+		$context->registerEventListener(BeforeZipCreatedEvent::class, BeforeZipCreatedListener::class, 5);
+		$context->registerEventListener(BeforeDirectFileDownloadEvent::class, BeforeDirectFileDownloadListener::class, 5);
 
 		// File request auth
 		$context->registerEventListener(BeforeTemplateRenderedEvent::class, LoadPublicFileRequestAuthListener::class);

apps/files_sharing/lib/Controller/ShareController.php

@@ -9,7 +9,6 @@ namespace OCA\Files_Sharing\Controller;
 use OC\Security\CSP\ContentSecurityPolicy;
 use OCA\DAV\Connector\Sabre\PublicAuth;
 use OCA\FederatedFileSharing\FederatedShareProvider;
-use OCA\Files_Sharing\Activity\Providers\Downloads;
 use OCA\Files_Sharing\Event\BeforeTemplateRenderedEvent;
 use OCA\Files_Sharing\Event\ShareLinkAccessedEvent;
 use OCP\Accounts\IAccountManager;
@@ -28,7 +27,6 @@ use OCP\EventDispatcher\IEventDispatcher;
 use OCP\Files\File;
 use OCP\Files\Folder;
 use OCP\Files\IRootFolder;
-use OCP\Files\Node;
 use OCP\Files\NotFoundException;
 use OCP\HintException;
 use OCP\IConfig;
@@ -368,15 +366,9 @@ class ShareController extends AuthPublicShareController {
 			throw new NotFoundException();
 		}
 
-		// Single file share
-		if ($share->getNode() instanceof File) {
-			// Single file download
-			$this->singleFileDownloaded($share, $share->getNode());
-		}
-		// Directory share
-		else {
-			/** @var Folder $node */
-			$node = $share->getNode();
+		$node = $share->getNode();
+		if ($node instanceof Folder) {
+			// Directory share
 
 			// Try to get the path
 			if ($path !== '') {
@@ -391,22 +383,10 @@ class ShareController extends AuthPublicShareController {
 
 			if ($node instanceof Folder) {
 				if ($files === null || $files === '') {
-					// The folder is downloaded
-					$this->singleFileDownloaded($share, $share->getNode());
-				} else {
-					$fileList = json_decode($files);
-					// in case we get only a single file
-					if (!is_array($fileList)) {
-						$fileList = [$fileList];
-					}
-					foreach ($fileList as $file) {
-						$subNode = $node->get($file);
-						$this->singleFileDownloaded($share, $subNode);
+					if ($share->getHideDownload()) {
+						throw new NotFoundException('Downloading a folder');
 					}
 				}
-			} else {
-				// Single file download
-				$this->singleFileDownloaded($share, $share->getNode());
 			}
 		}
 
@@ -419,77 +399,4 @@ class ShareController extends AuthPublicShareController {
 		}
 		return new RedirectResponse($this->urlGenerator->getAbsoluteURL($davUrl));
 	}
-
-	/**
-	 * create activity if a single file was downloaded from a link share
-	 *
-	 * @param Share\IShare $share
-	 * @throws NotFoundException when trying to download a folder of a "hide download" share
-	 */
-	protected function singleFileDownloaded(IShare $share, Node $node) {
-		if ($share->getHideDownload() && $node instanceof Folder) {
-			throw new NotFoundException('Downloading a folder');
-		}
-
-		$fileId = $node->getId();
-
-		$userFolder = $this->rootFolder->getUserFolder($share->getSharedBy());
-		$userNode = $userFolder->getFirstNodeById($fileId);
-		$ownerFolder = $this->rootFolder->getUserFolder($share->getShareOwner());
-		$userPath = $userFolder->getRelativePath($userNode->getPath());
-		$ownerPath = $ownerFolder->getRelativePath($node->getPath());
-		$remoteAddress = $this->request->getRemoteAddress();
-		$dateTime = new \DateTime();
-		$dateTime = $dateTime->format('Y-m-d H');
-		$remoteAddressHash = md5($dateTime . '-' . $remoteAddress);
-
-		$parameters = [$userPath];
-
-		if ($share->getShareType() === IShare::TYPE_EMAIL) {
-			if ($node instanceof File) {
-				$subject = Downloads::SUBJECT_SHARED_FILE_BY_EMAIL_DOWNLOADED;
-			} else {
-				$subject = Downloads::SUBJECT_SHARED_FOLDER_BY_EMAIL_DOWNLOADED;
-			}
-			$parameters[] = $share->getSharedWith();
-		} else {
-			if ($node instanceof File) {
-				$subject = Downloads::SUBJECT_PUBLIC_SHARED_FILE_DOWNLOADED;
-				$parameters[] = $remoteAddressHash;
-			} else {
-				$subject = Downloads::SUBJECT_PUBLIC_SHARED_FOLDER_DOWNLOADED;
-				$parameters[] = $remoteAddressHash;
-			}
-		}
-
-		$this->publishActivity($subject, $parameters, $share->getSharedBy(), $fileId, $userPath);
-
-		if ($share->getShareOwner() !== $share->getSharedBy()) {
-			$parameters[0] = $ownerPath;
-			$this->publishActivity($subject, $parameters, $share->getShareOwner(), $fileId, $ownerPath);
-		}
-	}
-
-	/**
-	 * publish activity
-	 *
-	 * @param string $subject
-	 * @param array $parameters
-	 * @param string $affectedUser
-	 * @param int $fileId
-	 * @param string $filePath
-	 */
-	protected function publishActivity($subject,
-		array $parameters,
-		$affectedUser,
-		$fileId,
-		$filePath) {
-		$event = $this->activityManager->generateEvent();
-		$event->setApp('files_sharing')
-			->setType('public_links')
-			->setSubject($subject, $parameters)
-			->setAffectedUser($affectedUser)
-			->setObject('files', $fileId, $filePath);
-		$this->activityManager->publish($event);
-	}
 }

apps/files_sharing/lib/Listener/BeforeNodeReadListener.php

@@ -0,0 +1,189 @@
+<?php
+
+declare(strict_types=1);
+
+/**
+ * SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-License-Identifier: AGPL-3.0-or-later
+ */
+
+namespace OCA\Files_Sharing\Listener;
+
+use OCA\Files_Sharing\Activity\Providers\Downloads;
+use OCP\EventDispatcher\Event;
+use OCP\EventDispatcher\IEventListener;
+use OCP\Files\Events\BeforeZipCreatedEvent;
+use OCP\Files\Events\Node\BeforeNodeReadEvent;
+use OCP\Files\File;
+use OCP\Files\Folder;
+use OCP\Files\IRootFolder;
+use OCP\Files\NotFoundException;
+use OCP\Files\Storage\ISharedStorage;
+use OCP\ICache;
+use OCP\ICacheFactory;
+use OCP\IRequest;
+use OCP\ISession;
+use OCP\Share\IShare;
+
+/**
+ * @template-implements IEventListener<BeforeNodeReadEvent|BeforeZipCreatedEvent|Event>
+ */
+class BeforeNodeReadListener implements IEventListener {
+	private ICache $cache;
+
+	public function __construct(
+		private ISession $session,
+		private IRootFolder $rootFolder,
+		private \OCP\Activity\IManager $activityManager,
+		private IRequest $request,
+		ICacheFactory $cacheFactory,
+	) {
+		$this->cache = $cacheFactory->createDistributed('files_sharing_activity_events');
+	}
+
+	public function handle(Event $event): void {
+		if ($event instanceof BeforeZipCreatedEvent) {
+			$this->handleBeforeZipCreatedEvent($event);
+		} elseif ($event instanceof BeforeNodeReadEvent) {
+			$this->handleBeforeNodeReadEvent($event);
+		}
+	}
+
+	public function handleBeforeZipCreatedEvent(BeforeZipCreatedEvent $event): void {
+		$files = $event->getFiles();
+		if (count($files) !== 0) {
+			/* No need to do anything, activity will be triggered for each file in the zip by the BeforeNodeReadEvent */
+			return;
+		}
+
+		$node = $event->getFolder();
+		if (!($node instanceof Folder)) {
+			return;
+		}
+
+		try {
+			$storage = $node->getStorage();
+		} catch (NotFoundException) {
+			return;
+		}
+
+		if (!$storage->instanceOfStorage(ISharedStorage::class)) {
+			return;
+		}
+
+		/** @var ISharedStorage $storage */
+		$share = $storage->getShare();
+
+		if (!in_array($share->getShareType(), [IShare::TYPE_EMAIL, IShare::TYPE_LINK])) {
+			return;
+		}
+
+		/* Cache that that folder download activity was published */
+		$this->cache->set($this->request->getId(), $node->getPath(), 3600);
+
+		$this->singleFileDownloaded($share, $node);
+	}
+
+	public function handleBeforeNodeReadEvent(BeforeNodeReadEvent $event): void {
+		$node = $event->getNode();
+		if (!($node instanceof File)) {
+			return;
+		}
+
+		try {
+			$storage = $node->getStorage();
+		} catch (NotFoundException) {
+			return;
+		}
+
+		if (!$storage->instanceOfStorage(ISharedStorage::class)) {
+			return;
+		}
+
+		/** @var ISharedStorage $storage */
+		$share = $storage->getShare();
+
+		if (!in_array($share->getShareType(), [IShare::TYPE_EMAIL, IShare::TYPE_LINK])) {
+			return;
+		}
+
+		$path = $this->cache->get($this->request->getId());
+		if (is_string($path) && str_starts_with($node->getPath(), $path)) {
+			/* An activity was published for a containing folder already */
+			return;
+		}
+
+		/* Avoid publishing several activities for one video playing */
+		$cacheKey = $node->getId() . $node->getPath() . $this->session->getId();
+		if (($this->request->getHeader('range') !== '') && ($this->cache->get($cacheKey) === 'true')) {
+			/* This is a range request and an activity for the same file was published in the same session */
+			return;
+		}
+		$this->cache->set($cacheKey, 'true', 3600);
+
+		$this->singleFileDownloaded($share, $node);
+	}
+
+	/**
+	 * create activity if a single file or folder was downloaded from a link share
+	 */
+	protected function singleFileDownloaded(IShare $share, File|Folder $node): void {
+		$fileId = $node->getId();
+
+		$userFolder = $this->rootFolder->getUserFolder($share->getSharedBy());
+		$userNode = $userFolder->getFirstNodeById($fileId);
+		$ownerFolder = $this->rootFolder->getUserFolder($share->getShareOwner());
+		$userPath = $userFolder->getRelativePath($userNode?->getPath() ?? '') ?? '';
+		$ownerPath = $ownerFolder->getRelativePath($node->getPath()) ?? '';
+
+		$parameters = [$userPath];
+
+		if ($share->getShareType() === IShare::TYPE_EMAIL) {
+			if ($node instanceof File) {
+				$subject = Downloads::SUBJECT_SHARED_FILE_BY_EMAIL_DOWNLOADED;
+			} else {
+				$subject = Downloads::SUBJECT_SHARED_FOLDER_BY_EMAIL_DOWNLOADED;
+			}
+			$parameters[] = $share->getSharedWith();
+		} elseif ($share->getShareType() === IShare::TYPE_LINK) {
+			if ($node instanceof File) {
+				$subject = Downloads::SUBJECT_PUBLIC_SHARED_FILE_DOWNLOADED;
+			} else {
+				$subject = Downloads::SUBJECT_PUBLIC_SHARED_FOLDER_DOWNLOADED;
+			}
+			$remoteAddress = $this->request->getRemoteAddress();
+			$dateTime = new \DateTime();
+			$dateTime = $dateTime->format('Y-m-d H');
+			$remoteAddressHash = md5($dateTime . '-' . $remoteAddress);
+			$parameters[] = $remoteAddressHash;
+		} else {
+			return;
+		}
+
+		$this->publishActivity($subject, $parameters, $share->getSharedBy(), $fileId, $userPath);
+
+		if ($share->getShareOwner() !== $share->getSharedBy()) {
+			$parameters[0] = $ownerPath;
+			$this->publishActivity($subject, $parameters, $share->getShareOwner(), $fileId, $ownerPath);
+		}
+	}
+
+	/**
+	 * publish activity
+	 */
+	protected function publishActivity(
+		string $subject,
+		array $parameters,
+		string $affectedUser,
+		int $fileId,
+		string $filePath,
+	): void {
+		$event = $this->activityManager->generateEvent();
+		$event->setApp('files_sharing')
+			->setType('public_links')
+			->setSubject($subject, $parameters)
+			->setAffectedUser($affectedUser)
+			->setObject('files', $fileId, $filePath);
+		$this->activityManager->publish($event);
+	}
+}

apps/sharebymail/lib/Activity.php

@@ -233,12 +233,12 @@ class Activity implements IProvider {
 	/**
 	 * @param int $id
 	 * @param string $path
-	 * @return array
+	 * @return array<string,string>
 	 */
-	protected function generateFileParameter($id, $path) {
+	protected function generateFileParameter($id, $path): array {
 		return [
 			'type' => 'file',
-			'id' => $id,
+			'id' => (string)$id,
 			'name' => basename($path),
 			'path' => trim($path, '/'),
 			'link' => $this->url->linkToRouteAbsolute('files.viewcontroller.showFile', ['fileid' => $id]),

build/integration/features/bootstrap/Activity.php

@@ -0,0 +1,32 @@
+<?php
+
+declare(strict_types=1);
+
+/**
+ * SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-License-Identifier: AGPL-3.0-or-later
+ */
+
+use Behat\Gherkin\Node\TableNode;
+use PHPUnit\Framework\Assert;
+
+trait Activity {
+	use BasicStructure;
+
+	/**
+	 * @Then last activity should be
+	 * @param TableNode $activity
+	 */
+	public function lastActivityIs(TableNode $activity): void {
+		$this->sendRequestForJSON('GET', '/apps/activity/api/v2/activity');
+		$this->theHTTPStatusCodeShouldBe('200');
+		$data = json_decode($this->response->getBody()->getContents(), true);
+		$activities = $data['ocs']['data'];
+		/* Sort by id */
+		uasort($activities, fn ($a, $b) => $a['activity_id'] <=> $b['activity_id']);
+		$lastActivity = array_pop($activities);
+		foreach ($activity->getRowsHash() as $key => $value) {
+			Assert::assertEquals($value, $lastActivity[$key]);
+		}
+	}
+}

build/integration/features/bootstrap/BasicStructure.php

@@ -192,8 +192,8 @@ trait BasicStructure {
 		$options = [];
 		if ($this->currentUser === 'admin') {
 			$options['auth'] = ['admin', 'admin'];
-		} elseif (strpos($this->currentUser, 'guest') !== 0) {
-			$options['auth'] = [$this->currentUser, self::TEST_PASSWORD];
+		} elseif (strpos($this->currentUser, 'anonymous') !== 0) {
+			$options['auth'] = [$this->currentUser, $this->regularUser];
 		}
 		if ($body instanceof TableNode) {
 			$fd = $body->getRowsHash();

build/integration/features/bootstrap/SharingContext.php

@@ -17,6 +17,7 @@ class SharingContext implements Context, SnippetAcceptingContext {
 	use Trashbin;
 	use AppConfiguration;
 	use CommandLine;
+	use Activity;
 
 	protected function resetAppConfigs() {
 		$this->deleteServerConfig('core', 'shareapi_default_permissions');

build/integration/sharing_features/sharing-activity.feature

@@ -0,0 +1,46 @@
+# SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors
+# SPDX-License-Identifier: AGPL-3.0-or-later
+Feature: sharing
+  Background:
+    Given using api version "1"
+    Given using new dav path
+    Given invoking occ with "app:enable --force activity"
+    Given the command was successful
+    Given user "user0" exists
+    And Logging in using web as "user0"
+    And Sending a "POST" to "/apps/activity/settings" with requesttoken
+      | public_links_notification | 1 |
+      | public_links_upload_notification | 1 |
+      | notify_setting_batchtime | 0 |
+      | activity_digest | 0 |
+
+  Scenario: Creating a new mail share and check activity
+    Given dummy mail server is listening
+    And As an "user0"
+    When creating a share with
+      | path | welcome.txt |
+      | shareType | 4 |
+      | shareWith | dumy@test.com |
+    Then the OCS status code should be "100"
+    And the HTTP status code should be "200"
+    And last share can be downloaded
+    Then last activity should be
+      | app | files_sharing |
+      | type | public_links |
+      | object_type | files |
+      | object_name | /welcome.txt |
+
+  Scenario: Creating a new public share and check activity
+    Given user "user0" exists
+    And As an "user0"
+    When creating a share with
+      | path | welcome.txt |
+      | shareType | 3 |
+    Then the OCS status code should be "100"
+    And the HTTP status code should be "200"
+    And last link share can be downloaded
+    Then last activity should be
+      | app | files_sharing |
+      | type | public_links |
+      | object_type | files |
+      | object_name | /welcome.txt |