Skip to content
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.

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

fix(attachments): Don't allow selecting shared folders as attachment … #11426

Merged
merged 1 commit into from
Jan 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 2 additions & 1 deletion lib/Controller/PageController.php
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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;
Expand Down
3 changes: 3 additions & 0 deletions lib/Controller/SettingsController.php
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,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);
Expand Down
3 changes: 3 additions & 0 deletions lib/Files/TemplateLoader.php
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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;
}

/**
Expand Down
3 changes: 3 additions & 0 deletions lib/PublicShare/TemplateLoader.php
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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;
}

/**
Expand Down
3 changes: 3 additions & 0 deletions lib/PublicShareAuth/TemplateLoader.php
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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;
}

/**
Expand Down
7 changes: 7 additions & 0 deletions lib/Service/RecordingService.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
8 changes: 8 additions & 0 deletions lib/TInitialState.php
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
use OCP\IGroupManager;
use OCP\IUser;
use OCP\Util;
use Psr\Log\LoggerInterface;

trait TInitialState {
/** @var Config */
Expand All @@ -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
Expand Down Expand Up @@ -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);
}
Expand Down
14 changes: 14 additions & 0 deletions tests/integration/features/sharing-4/settings.feature
Original file line number Diff line number Diff line change
@@ -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"
Loading