From a3702a1bd6900a504ce4cbe67e4d3075ab5e9260 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julius=20H=C3=A4rtl?= Date: Mon, 26 Aug 2024 15:29:00 +0200 Subject: [PATCH 1/4] fix: Add brute force protection for public file creation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Julius Härtl --- lib/Controller/DocumentAPIController.php | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/Controller/DocumentAPIController.php b/lib/Controller/DocumentAPIController.php index 74f0c4446f..ebc5f9c9d6 100644 --- a/lib/Controller/DocumentAPIController.php +++ b/lib/Controller/DocumentAPIController.php @@ -74,6 +74,7 @@ public function __construct(IRequest $request, IRootFolder $rootFolder, IManager * * @NoAdminRequired * @PublicPage + * @BruteForceProtection(action=richdocumentsCreatePublic) */ public function create(string $mimeType, string $fileName, string $directoryPath = '/', ?string $shareToken = null, ?int $templateId = null): JSONResponse { try { From 57cb3b346af0be8e7de736ec26d793fba6ec5fba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julius=20H=C3=A4rtl?= Date: Mon, 26 Aug 2024 15:30:27 +0200 Subject: [PATCH 2/4] fix: Check for update permissions when creating a new file MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Julius Härtl --- lib/Controller/DocumentAPIController.php | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/lib/Controller/DocumentAPIController.php b/lib/Controller/DocumentAPIController.php index ebc5f9c9d6..ea46dc982c 100644 --- a/lib/Controller/DocumentAPIController.php +++ b/lib/Controller/DocumentAPIController.php @@ -80,6 +80,12 @@ public function create(string $mimeType, string $fileName, string $directoryPath try { if ($shareToken !== null) { $share = $this->shareManager->getShareByToken($shareToken); + if (!($share->getPermissions() & \OCP\Constants::PERMISSION_CREATE)) { + return new JSONResponse([ + 'status' => 'error', + 'message' => $this->l10n->t('Not allowed to create document') + ], Http::STATUS_FORBIDDEN); + } } $rootFolder = $shareToken !== null ? $share->getNode() : $this->rootFolder->getUserFolder($this->userId); From bd4118b860ca353114c08bb68cb7cbf4ffefe2c8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julius=20H=C3=A4rtl?= Date: Mon, 26 Aug 2024 15:43:28 +0200 Subject: [PATCH 3/4] fix: Check for share link authentication MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Julius Härtl --- lib/Controller/DocumentAPIController.php | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/lib/Controller/DocumentAPIController.php b/lib/Controller/DocumentAPIController.php index ea46dc982c..a907d3b6ee 100644 --- a/lib/Controller/DocumentAPIController.php +++ b/lib/Controller/DocumentAPIController.php @@ -40,6 +40,7 @@ use OCP\Files\Lock\NoLockProviderException; use OCP\IL10N; use OCP\IRequest; +use OCP\ISession; use OCP\PreConditionNotMetException; use OCP\Share\IManager; use Psr\Log\LoggerInterface; @@ -52,9 +53,10 @@ class DocumentAPIController extends \OCP\AppFramework\OCSController { private $l10n; private $logger; private $lockManager; + private $session; private $userId; - public function __construct(IRequest $request, IRootFolder $rootFolder, IManager $shareManager, TemplateManager $templateManager, IL10N $l10n, LoggerInterface $logger, ILockManager $lockManager, $userId) { + public function __construct(IRequest $request, IRootFolder $rootFolder, IManager $shareManager, TemplateManager $templateManager, IL10N $l10n, LoggerInterface $logger, ILockManager $lockManager, ISession $session, $userId) { parent::__construct(Application::APPNAME, $request); $this->rootFolder = $rootFolder; $this->shareManager = $shareManager; @@ -62,6 +64,7 @@ public function __construct(IRequest $request, IRootFolder $rootFolder, IManager $this->l10n = $l10n; $this->logger = $logger; $this->lockManager = $lockManager; + $this->session = $session; $this->userId = $userId; } @@ -80,11 +83,17 @@ public function create(string $mimeType, string $fileName, string $directoryPath try { if ($shareToken !== null) { $share = $this->shareManager->getShareByToken($shareToken); + + if ($share->getPassword()) { + if (!$this->session->exists('public_link_authenticated') + || $this->session->get('public_link_authenticated') !== (string)$share->getId() + ) { + throw new Exception('Invalid password'); + } + } + if (!($share->getPermissions() & \OCP\Constants::PERMISSION_CREATE)) { - return new JSONResponse([ - 'status' => 'error', - 'message' => $this->l10n->t('Not allowed to create document') - ], Http::STATUS_FORBIDDEN); + throw new Exception('No create permissions'); } } From 0c0913e20f339b33969b9b566a98a7d5bc2585ed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julius=20H=C3=A4rtl?= Date: Mon, 26 Aug 2024 15:46:07 +0200 Subject: [PATCH 4/4] chore: Use attributes for method annotation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Julius Härtl --- lib/Controller/DocumentAPIController.php | 41 +++++++++++------------- 1 file changed, 18 insertions(+), 23 deletions(-) diff --git a/lib/Controller/DocumentAPIController.php b/lib/Controller/DocumentAPIController.php index a907d3b6ee..b6a495bc13 100644 --- a/lib/Controller/DocumentAPIController.php +++ b/lib/Controller/DocumentAPIController.php @@ -30,6 +30,9 @@ use OCA\Richdocuments\Helper; use OCA\Richdocuments\TemplateManager; use OCP\AppFramework\Http; +use OCP\AppFramework\Http\Attribute\BruteForceProtection; +use OCP\AppFramework\Http\Attribute\NoAdminRequired; +use OCP\AppFramework\Http\Attribute\PublicPage; use OCP\AppFramework\Http\DataResponse; use OCP\AppFramework\Http\JSONResponse; use OCP\Files\Folder; @@ -47,25 +50,18 @@ use Throwable; class DocumentAPIController extends \OCP\AppFramework\OCSController { - private $rootFolder; - private $shareManager; - private $templateManager; - private $l10n; - private $logger; - private $lockManager; - private $session; - private $userId; - - public function __construct(IRequest $request, IRootFolder $rootFolder, IManager $shareManager, TemplateManager $templateManager, IL10N $l10n, LoggerInterface $logger, ILockManager $lockManager, ISession $session, $userId) { + public function __construct( + IRequest $request, + private IRootFolder $rootFolder, + private IManager $shareManager, + private TemplateManager $templateManager, + private IL10N $l10n, + private LoggerInterface $logger, + private ILockManager $lockManager, + private ISession $session, + private ?string $userId + ) { parent::__construct(Application::APPNAME, $request); - $this->rootFolder = $rootFolder; - $this->shareManager = $shareManager; - $this->templateManager = $templateManager; - $this->l10n = $l10n; - $this->logger = $logger; - $this->lockManager = $lockManager; - $this->session = $session; - $this->userId = $userId; } /** @@ -74,11 +70,10 @@ public function __construct(IRequest $request, IRootFolder $rootFolder, IManager * As the server template API for file creation is not available there, we need a dedicated API * in order to properly create files as public page visitors. This is being called in the new file * actions in src/view/NewFileMenu.js - * - * @NoAdminRequired - * @PublicPage - * @BruteForceProtection(action=richdocumentsCreatePublic) */ + #[NoAdminRequired] + #[PublicPage] + #[BruteForceProtection(action: 'richdocumentsCreatePublic')] public function create(string $mimeType, string $fileName, string $directoryPath = '/', ?string $shareToken = null, ?int $templateId = null): JSONResponse { try { if ($shareToken !== null) { @@ -172,7 +167,7 @@ public function create(string $mimeType, string $fileName, string $directoryPath ]); } - #[Http\Attribute\NoAdminRequired] + #[NoAdminRequired] public function openLocal(int $fileId): DataResponse { try { $files = $this->rootFolder->getUserFolder($this->userId)->getById($fileId);