From ab873059762840bdcd05dbf8c16f9be9b1b8bbd1 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Mon, 22 Jan 2024 14:10:53 +0100 Subject: [PATCH] fix(attachments): Don't allow selecting shared folders as attachment folder Signed-off-by: Joas Schilling --- lib/Controller/PageController.php | 3 ++- lib/Controller/SettingsController.php | 3 +++ lib/Files/TemplateLoader.php | 3 +++ lib/PublicShare/TemplateLoader.php | 3 +++ lib/PublicShareAuth/TemplateLoader.php | 3 +++ lib/Service/RecordingService.php | 7 +++++++ lib/TInitialState.php | 8 ++++++++ .../features/sharing-4/settings.feature | 14 ++++++++++++++ 8 files changed, 43 insertions(+), 1 deletion(-) create mode 100644 tests/integration/features/sharing-4/settings.feature diff --git a/lib/Controller/PageController.php b/lib/Controller/PageController.php index 537bd4339cc..892b2df0efd 100644 --- a/lib/Controller/PageController.php +++ b/lib/Controller/PageController.php @@ -81,7 +81,7 @@ public function __construct( private TalkSession $talkSession, private IUserSession $userSession, private ?string $userId, - private LoggerInterface $logger, + LoggerInterface $logger, private Manager $manager, private ParticipantService $participantService, private RoomService $roomService, @@ -97,6 +97,7 @@ public function __construct( IGroupManager $groupManager, ) { parent::__construct($appName, $request); + $this->logger = $logger; $this->initialState = $initialState; $this->memcacheFactory = $memcacheFactory; $this->talkConfig = $talkConfig; diff --git a/lib/Controller/SettingsController.php b/lib/Controller/SettingsController.php index 245103de216..932c24917ec 100644 --- a/lib/Controller/SettingsController.php +++ b/lib/Controller/SettingsController.php @@ -97,6 +97,9 @@ protected function validateUserSetting(string $setting, $value): bool { if (!$node instanceof Folder) { throw new NotPermittedException('Node is not a directory'); } + if ($node->isShared()) { + throw new NotPermittedException('Folder is shared'); + } return !$node->getStorage()->instanceOfStorage(SharedStorage::class); } catch (NotFoundException $e) { $userFolder->newFolder($value); diff --git a/lib/Files/TemplateLoader.php b/lib/Files/TemplateLoader.php index 7f46f01b675..7aa68315a28 100644 --- a/lib/Files/TemplateLoader.php +++ b/lib/Files/TemplateLoader.php @@ -40,6 +40,7 @@ use OCP\IUser; use OCP\IUserSession; use OCP\Util; +use Psr\Log\LoggerInterface; /** * Helper class to add the Talk UI to the sidebar of the Files app. @@ -59,12 +60,14 @@ public function __construct( private IUserSession $userSession, IGroupManager $groupManager, protected IRequest $request, + LoggerInterface $logger, ) { $this->initialState = $initialState; $this->memcacheFactory = $memcacheFactory; $this->talkConfig = $talkConfig; $this->serverConfig = $serverConfig; $this->groupManager = $groupManager; + $this->logger = $logger; } /** diff --git a/lib/PublicShare/TemplateLoader.php b/lib/PublicShare/TemplateLoader.php index 0274c7a32d9..0ba3546b280 100644 --- a/lib/PublicShare/TemplateLoader.php +++ b/lib/PublicShare/TemplateLoader.php @@ -37,6 +37,7 @@ use OCP\IConfig; use OCP\IGroupManager; use OCP\Util; +use Psr\Log\LoggerInterface; /** * Helper class to extend the "publicshare" template from the server. @@ -54,12 +55,14 @@ public function __construct( Config $talkConfig, IConfig $serverConfig, IGroupManager $groupManager, + LoggerInterface $logger, ) { $this->initialState = $initialState; $this->talkConfig = $talkConfig; $this->memcacheFactory = $memcacheFactory; $this->serverConfig = $serverConfig; $this->groupManager = $groupManager; + $this->logger = $logger; } /** diff --git a/lib/PublicShareAuth/TemplateLoader.php b/lib/PublicShareAuth/TemplateLoader.php index 8dec68cbe0e..b2201896601 100644 --- a/lib/PublicShareAuth/TemplateLoader.php +++ b/lib/PublicShareAuth/TemplateLoader.php @@ -36,6 +36,7 @@ use OCP\IConfig; use OCP\IGroupManager; use OCP\Util; +use Psr\Log\LoggerInterface; /** * Helper class to extend the "publicshareauth" template from the server. @@ -53,12 +54,14 @@ public function __construct( Config $talkConfig, IConfig $serverConfig, IGroupManager $groupManager, + LoggerInterface $logger, ) { $this->initialState = $initialState; $this->talkConfig = $talkConfig; $this->memcacheFactory = $memcacheFactory; $this->serverConfig = $serverConfig; $this->groupManager = $groupManager; + $this->logger = $logger; } /** diff --git a/lib/Service/RecordingService.php b/lib/Service/RecordingService.php index 509be38d4f8..999514cc9a1 100644 --- a/lib/Service/RecordingService.php +++ b/lib/Service/RecordingService.php @@ -289,6 +289,13 @@ private function getRecordingFolder(string $owner, string $token): Folder { try { /** @var \OCP\Files\Folder */ $recordingRootFolder = $userFolder->get($recordingRootFolderName); + if ($recordingRootFolder->isShared()) { + $this->logger->error('Talk attachment folder for user {userId} is set to a shared folder. Resetting to their root.', [ + 'userId' => $owner, + ]); + + $this->serverConfig->setUserValue($owner, 'spreed', 'attachment_folder', '/'); + } } catch (NotFoundException $e) { /** @var \OCP\Files\Folder */ $recordingRootFolder = $userFolder->newFolder($recordingRootFolderName); diff --git a/lib/TInitialState.php b/lib/TInitialState.php index 2df881eed63..fe28da8a589 100644 --- a/lib/TInitialState.php +++ b/lib/TInitialState.php @@ -35,6 +35,7 @@ use OCP\IGroupManager; use OCP\IUser; use OCP\Util; +use Psr\Log\LoggerInterface; trait TInitialState { /** @var Config */ @@ -46,6 +47,7 @@ trait TInitialState { /** @var ICacheFactory */ protected $memcacheFactory; protected IGroupManager $groupManager; + protected LoggerInterface $logger; protected function publishInitialStateShared(): void { // Needed to enable the screensharing extension in Chromium < 72 @@ -147,6 +149,12 @@ protected function publishInitialStateForUser(IUser $user, IRootFolder $rootFold try { try { $folder = $userFolder->get($attachmentFolder); + if ($folder->isShared()) { + $this->logger->error('Talk attachment folder for user {userId} is set to a shared folder. Resetting to their root.', [ + 'userId' => $user->getUID(), + ]); + throw new NotPermittedException('Folder is shared'); + } } catch (NotFoundException $e) { $folder = $userFolder->newFolder($attachmentFolder); } diff --git a/tests/integration/features/sharing-4/settings.feature b/tests/integration/features/sharing-4/settings.feature new file mode 100644 index 00000000000..40e76e460d5 --- /dev/null +++ b/tests/integration/features/sharing-4/settings.feature @@ -0,0 +1,14 @@ +Feature: sharing-4/settings + + Background: + Given user "participant1" exists + Given user "participant2" exists + + Scenario: Do not allow setting a shared folder as attachment_folder + Given user "participant1" creates folder "/test" + When user "participant1" sets setting "attachment_folder" to "/test" with 200 (v1) + Then user "participant1" has capability "spreed=>config=>attachments=>folder" set to "/test" + Given user "participant2" creates folder "/test-participant2" + Given user "participant2" shares "/test-participant2" with user "participant1" with OCS 100 + When user "participant1" sets setting "attachment_folder" to "/test-participant2" with 400 (v1) + Then user "participant1" has capability "spreed=>config=>attachments=>folder" set to "/test"