Merge pull request #22423 from nextcloud/bugfix/noid/direct-editing-e…

…ncryption Do not expose direct editing if no master key is available
nextcloud · Sep 1, 2020 · 6bda2c2 · 6bda2c2
2 parents 2b192e7 + e0ae377
commit 6bda2c2
Show file tree

Hide file tree

Showing 5 changed files with 60 additions and 10 deletions.
diff --git a/apps/files/lib/Controller/DirectEditingController.php b/apps/files/lib/Controller/DirectEditingController.php
@@ -76,6 +76,9 @@ public function info(): DataResponse {
 	 * @NoAdminRequired
 	 */
 	public function create(string $path, string $editorId, string $creatorId, string $templateId = null): DataResponse {
+		if (!$this->directEditingManager->isEnabled()) {
+			return new DataResponse(['message' => 'Direct editing is not enabled'], Http::STATUS_INTERNAL_SERVER_ERROR);
+		}
 		$this->eventDispatcher->dispatchTyped(new RegisterDirectEditorEvent($this->directEditingManager));
 
 		try {
@@ -85,14 +88,17 @@ public function create(string $path, string $editorId, string $creatorId, string
 			]);
 		} catch (Exception $e) {
 			$this->logger->logException($e, ['message' => 'Exception when creating a new file through direct editing']);
-			return new DataResponse('Failed to create file: ' . $e->getMessage(), Http::STATUS_FORBIDDEN);
+			return new DataResponse(['message' => 'Failed to create file: ' . $e->getMessage()], Http::STATUS_FORBIDDEN);
 		}
 	}
 
 	/**
 	 * @NoAdminRequired
 	 */
 	public function open(string $path, string $editorId = null): DataResponse {
+		if (!$this->directEditingManager->isEnabled()) {
+			return new DataResponse(['message' => 'Direct editing is not enabled'], Http::STATUS_INTERNAL_SERVER_ERROR);
+		}
 		$this->eventDispatcher->dispatchTyped(new RegisterDirectEditorEvent($this->directEditingManager));
 
 		try {
@@ -102,7 +108,7 @@ public function open(string $path, string $editorId = null): DataResponse {
 			]);
 		} catch (Exception $e) {
 			$this->logger->logException($e, ['message' => 'Exception when opening a file through direct editing']);
-			return new DataResponse('Failed to open file: ' . $e->getMessage(), Http::STATUS_FORBIDDEN);
+			return new DataResponse(['message' => 'Failed to open file: ' . $e->getMessage()], Http::STATUS_FORBIDDEN);
 		}
 	}
 
@@ -112,13 +118,16 @@ public function open(string $path, string $editorId = null): DataResponse {
 	 * @NoAdminRequired
 	 */
 	public function templates(string $editorId, string $creatorId): DataResponse {
+		if (!$this->directEditingManager->isEnabled()) {
+			return new DataResponse(['message' => 'Direct editing is not enabled'], Http::STATUS_INTERNAL_SERVER_ERROR);
+		}
 		$this->eventDispatcher->dispatchTyped(new RegisterDirectEditorEvent($this->directEditingManager));
 
 		try {
 			return new DataResponse($this->directEditingManager->getTemplates($editorId, $creatorId));
 		} catch (Exception $e) {
 			$this->logger->logException($e);
-			return new DataResponse('Failed to obtain template list: ' . $e->getMessage(), Http::STATUS_INTERNAL_SERVER_ERROR);
+			return new DataResponse(['message' => 'Failed to obtain template list: ' . $e->getMessage()], Http::STATUS_INTERNAL_SERVER_ERROR);
 		}
 	}
 }
diff --git a/apps/files/lib/Service/DirectEditingService.php b/apps/files/lib/Service/DirectEditingService.php
@@ -55,6 +55,10 @@ public function getDirectEditingCapabilitites(): array {
 			'creators' => []
 		];
 
+		if (!$this->directEditingManager->isEnabled()) {
+			return $capabilities;
+		}
+
 		/**
 		 * @var string $id
 		 * @var IEditor $editor

diff --git a/lib/private/DirectEditing/Manager.php b/lib/private/DirectEditing/Manager.php
@@ -35,6 +35,7 @@
 use OCP\DirectEditing\IEditor;
 use \OCP\DirectEditing\IManager;
 use OCP\DirectEditing\IToken;
+use OCP\Encryption\IManager as EncryptionManager;
 use OCP\Files\File;
 use OCP\Files\IRootFolder;
 use OCP\Files\Node;
@@ -45,6 +46,7 @@
 use OCP\L10N\IFactory;
 use OCP\Security\ISecureRandom;
 use OCP\Share\IShare;
+use Throwable;
 use function array_key_exists;
 use function in_array;
 
@@ -55,30 +57,33 @@ class Manager implements IManager {
 
 	/** @var IEditor[] */
 	private $editors = [];
-
 	/** @var IDBConnection */
 	private $connection;
-	/**
-	 * @var ISecureRandom
-	 */
+	/** @var ISecureRandom */
 	private $random;
+	/** @var string|null */
 	private $userId;
+	/** @var IRootFolder */
 	private $rootFolder;
 	/** @var IL10N */
 	private $l10n;
+	/** @var EncryptionManager */
+	private $encryptionManager;
 
 	public function __construct(
 		ISecureRandom $random,
 		IDBConnection $connection,
 		IUserSession $userSession,
 		IRootFolder $rootFolder,
-		IFactory $l10nFactory
+		IFactory $l10nFactory,
+		EncryptionManager $encryptionManager
 	) {
 		$this->random = $random;
 		$this->connection = $connection;
 		$this->userId = $userSession->getUser() ? $userSession->getUser()->getUID() : null;
 		$this->rootFolder = $rootFolder;
 		$this->l10n = $l10nFactory->get('core');
+		$this->encryptionManager = $encryptionManager;
 	}
 
 	public function registerDirectEditor(IEditor $directEditor): void {
@@ -171,7 +176,7 @@ public function edit(string $token): Response {
 			}
 			$editor = $this->getEditor($tokenObject->getEditor());
 			$this->accessToken($token);
-		} catch (\Throwable $throwable) {
+		} catch (Throwable $throwable) {
 			$this->invalidateToken($token);
 			return new NotFoundResponse();
 		}
@@ -275,4 +280,22 @@ public function getFileForToken($userId, $fileId, $filePath = null): Node {
 		}
 		return $files[0];
 	}
+
+	public function isEnabled(): bool {
+		if (!$this->encryptionManager->isEnabled()) {
+			return true;
+		}
+
+		try {
+			$moduleId = $this->encryptionManager->getDefaultEncryptionModuleId();
+			$module = $this->encryptionManager->getEncryptionModule($moduleId);
+			/** @var \OCA\Encryption\Util $util */
+			$util = \OC::$server->get(\OCA\Encryption\Util::class);
+			if ($module->isReadyForUser($this->userId) && $util->isMasterKeyEnabled()) {
+				return true;
+			}
+		} catch (Throwable $e) {
+		}
+		return false;
+	}
 }
diff --git a/lib/public/DirectEditing/IManager.php b/lib/public/DirectEditing/IManager.php
@@ -84,4 +84,12 @@ public function getToken(string $token): IToken;
 	 * @return int number of deleted tokens
 	 */
 	public function cleanup(): int;
+
+	/**
+	 * Check if direct editing is enabled
+	 *
+	 * @since 20.0.0
+	 * @return bool
+	 */
+	public function isEnabled(): bool;
 }
diff --git a/tests/lib/DirectEditing/ManagerTest.php b/tests/lib/DirectEditing/ManagerTest.php
@@ -10,6 +10,7 @@
 use OCP\DirectEditing\ACreateEmpty;
 use OCP\DirectEditing\IEditor;
 use OCP\DirectEditing\IToken;
+use OCP\Encryption\IManager;
 use OCP\Files\Folder;
 use OCP\Files\IRootFolder;
 use OCP\IDBConnection;
@@ -104,6 +105,10 @@ class ManagerTest extends TestCase {
 	 * @var MockObject|Folder
 	 */
 	private $userFolder;
+	/**
+	 * @var MockObject|IManager
+	 */
+	private $encryptionManager;
 
 	protected function setUp(): void {
 		parent::setUp();
@@ -116,6 +121,7 @@ protected function setUp(): void {
 		$this->rootFolder = $this->createMock(IRootFolder::class);
 		$this->userFolder = $this->createMock(Folder::class);
 		$this->l10n = $this->createMock(IL10N::class);
+		$this->encryptionManager = $this->createMock(IManager::class);
 
 		$l10nFactory = $this->createMock(IFactory::class);
 		$l10nFactory->expects($this->once())
@@ -128,7 +134,7 @@ protected function setUp(): void {
 			->willReturn($this->userFolder);
 
 		$this->manager = new Manager(
-			$this->random, $this->connection, $this->userSession, $this->rootFolder, $l10nFactory
+			$this->random, $this->connection, $this->userSession, $this->rootFolder, $l10nFactory, $this->encryptionManager
 		);
 
 		$this->manager->registerDirectEditor($this->editor);