Skip to content

Fix public download activity #51458

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
susnux merged 9 commits into from
Mar 25, 2025
Merged

Fix public download activity #51458

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
12ce8c0
fix(sharing): Publish activity for download by public link
come-nc Mar 13, 2025
a39bee5
fix: Fix download activity for folders
come-nc Mar 13, 2025
a3c531c
fix: Avoid triggering several activities for Range request on the sam…
come-nc Mar 13, 2025
ec5ac09
fix(files_sharing): Set higher priority for listeners that may abort …
come-nc Mar 13, 2025
a32875d
fix(files_sharing): Use session id as part of cache key to avoid conc…
come-nc Mar 13, 2025
a57f694
feat: add integration test for sharing activity
come-nc Mar 18, 2025
140aba1
feat: Add context and test steps for activity in sharing
come-nc Mar 18, 2025
c9938e5
fix(sharebymail): Fix activity rich object id type
come-nc Mar 18, 2025
da9b6e3
fix(tests): Sort activities by id to get the last one
come-nc Mar 18, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
fix: Fix download activity for folders 
Remove duplicate activity publishing from share controller download,
 listen to BeforeZipCreatedEvent to publish activity for folders, and
 cache folders activity to avoid sending activity for each file in the
 folder.

Signed-off-by: Côme Chilliet <[email protected]>
  • Loading branch information
@come-nc
come-nc committed Mar 25, 2025
commit a39bee57d95ac89ac248f7cb28f49c4ca9c749dc
1 change: 1 addition & 0 deletions apps/files_sharing/lib/AppInfo/Application.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@ function () use ($c) {

// Publish activity for public download
$context->registerEventListener(BeforeNodeReadEvent::class, BeforeNodeReadListener::class);
$context->registerEventListener(BeforeZipCreatedEvent::class, BeforeNodeReadListener::class);

// Handle download events for view only checks
$context->registerEventListener(BeforeZipCreatedEvent::class, BeforeZipCreatedListener::class);
Expand Down
103 changes: 5 additions & 98 deletions apps/files_sharing/lib/Controller/ShareController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
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;
Expand All @@ -28,7 +27,6 @@
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;
Expand Down Expand Up @@ -368,15 +366,9 @@ public function downloadShare($token, $files = null, $path = '') {
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 !== '') {
Expand All @@ -391,22 +383,10 @@ public function downloadShare($token, $files = null, $path = '') {

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());
}
}

Expand All @@ -419,77 +399,4 @@ public function downloadShare($token, $files = null, $path = '') {
}
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);
}
}
79 changes: 72 additions & 7 deletions apps/files_sharing/lib/Listener/BeforeNodeReadListener.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,32 +12,79 @@
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\IUserSession;
use OCP\Share\IShare;

/**
* @template-implements IEventListener<BeforeNodeReadEvent|Event>
* @template-implements IEventListener<BeforeNodeReadEvent|BeforeZipCreatedEvent|Event>
*/
class BeforeNodeReadListener implements IEventListener {
private ICache $cache;

public function __construct(
private IUserSession $userSession,
private IRootFolder $rootFolder,
protected \OCP\Activity\IManager $activityManager,
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 BeforeNodeReadEvent)) {
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;
Expand All @@ -56,13 +103,23 @@ public function handle(Event $event): void {
/** @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;
}

$this->singleFileDownloaded($share, $node);
}

/**
* create activity if a single file was downloaded from a link share
* create activity if a single file or folder was downloaded from a link share
*/
protected function singleFileDownloaded(IShare $share, File $node): void {
protected function singleFileDownloaded(IShare $share, File|Folder $node): void {
$fileId = $node->getId();

$userFolder = $this->rootFolder->getUserFolder($share->getSharedBy());
Expand All @@ -74,10 +131,18 @@ protected function singleFileDownloaded(IShare $share, File $node): void {
$parameters = [$userPath];

if ($share->getShareType() === IShare::TYPE_EMAIL) {
$subject = Downloads::SUBJECT_SHARED_FILE_BY_EMAIL_DOWNLOADED;
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) {
$subject = Downloads::SUBJECT_PUBLIC_SHARED_FILE_DOWNLOADED;
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');
Expand Down