Feat/storage based encryption key location 2 10.9

CHANGELOG.md

@@ -1,4 +1,45 @@
-Changelog for ownCloud Core [10.9.0] (2021-12-20)
+Changelog for ownCloud Core [10.9.1] (2022-01-12)
+=======================================
+The following sections list the changes in ownCloud core 10.9.1 relevant to
+ownCloud admins and users.
+
+[10.9.1]: https://github.com/owncloud/core/compare/v10.9.0...v10.9.1
+
+Summary
+-------
+
+* Bugfix - Prevent encrypted files from being corrupted when overwriting them: [#39623](https://github.com/owncloud/core/pull/39623)
+* Bugfix - Getting the file owner for share recipients: [#39670](https://github.com/owncloud/core/pull/39670)
+* Bugfix - Prevent version author from being overwritten with wrong uid: [#39673](https://github.com/owncloud/core/pull/39673)
+
+Details
+-------
+
+* Bugfix - Prevent encrypted files from being corrupted when overwriting them: [#39623](https://github.com/owncloud/core/pull/39623)
+
+   Fixed an issue where overwriting an encrypted file by a share recipient would corrupt it. This
+   is a regression which was introduced by #39516.
+
+   https://github.com/owncloud/encryption/issues/315
+   https://github.com/owncloud/core/pull/39623
+
+* Bugfix - Getting the file owner for share recipients: [#39670](https://github.com/owncloud/core/pull/39670)
+
+   Fixed a bug where a wrong file owner was retrieved when saving version authors. This scenario
+   happened for share recipients if they had a file with the same name as the shared file.
+
+   https://github.com/owncloud/core/issues/39662
+   https://github.com/owncloud/core/pull/39670
+
+* Bugfix - Prevent version author from being overwritten with wrong uid: [#39673](https://github.com/owncloud/core/pull/39673)
+
+   Fixed an issue where restoring a previous version could lead to a wrong version author being
+   saved, basically overwriting the correct author.
+
+   https://github.com/owncloud/core/issues/39672
+   https://github.com/owncloud/core/pull/39673
+
+Changelog for ownCloud Core [10.9.0] (2021-12-09)
 =======================================
 The following sections list the changes in ownCloud core 10.9.0 relevant to
 ownCloud admins and users.

Makefile

@@ -378,8 +378,8 @@ changelog:
 	@echo This file is autogenerated on the master branch.
 	@echo Committing changes to this file will result in merge conflicts.
 	@echo ======================================================================
-	docker run --rm -v $(work_dir):$(work_dir) -w $(work_dir) toolhippie/calens >| CHANGELOG.md
-	docker run --rm -v $(work_dir):$(work_dir) -w $(work_dir) toolhippie/calens -t changelog/CHANGELOG-html.tmpl >| CHANGELOG.html
+	docker run --rm -v $(work_dir):$(work_dir) -w $(work_dir) toolhippie/calens calens >| CHANGELOG.md
+	docker run --rm -v $(work_dir):$(work_dir) -w $(work_dir) toolhippie/calens calens -t changelog/CHANGELOG-html.tmpl >| CHANGELOG.html
 
 #
 # Dependency management

apps/dav/lib/Connector/Sabre/File.php

@@ -187,9 +187,7 @@ public function put($data) {
 			try {
 				$this->changeLock(ILockingProvider::LOCK_EXCLUSIVE);
 			} catch (LockedException $e) {
-				if ($usePartFile) {
-					$partStorage->unlink($internalPartPath);
-				}
+				$this->cleanFailedUpload($partStorage, $internalPartPath);
 				throw new FileLocked($e->getMessage(), $e->getCode(), $e);
 			}
 
@@ -237,9 +235,7 @@ public function put($data) {
 				}
 			}
 		} catch (\Exception $e) {
-			if ($usePartFile) {
-				$partStorage->unlink($internalPartPath);
-			}
+			$this->cleanFailedUpload($partStorage, $internalPartPath);
 			$this->convertToSabreException($e);
 		}
 
@@ -258,9 +254,7 @@ public function put($data) {
 			try {
 				$this->changeLock(ILockingProvider::LOCK_EXCLUSIVE);
 			} catch (LockedException $e) {
-				if ($usePartFile) {
-					$partStorage->unlink($internalPartPath);
-				}
+				$this->cleanFailedUpload($partStorage, $internalPartPath);
 				throw new FileLocked($e->getMessage(), $e->getCode(), $e);
 			}
 
@@ -604,9 +598,7 @@ private function createFileChunked($data) {
 				}
 				return $etag;
 			} catch (\Exception $e) {
-				if ($partFile !== null) {
-					$targetStorage->unlink($targetInternalPath);
-				}
+				$this->cleanFailedUpload($targetStorage, $targetInternalPath);
 				$this->convertToSabreException($e);
 			}
 		}
@@ -738,4 +730,24 @@ protected function header($string) {
 	public function getNode() {
 		return \OC::$server->getRootFolder()->get($this->getFileInfo()->getPath());
 	}
+
+	private function cleanFailedUpload(Storage $partStorage, $internalPartPath): void {
+		if ($partStorage->file_exists($internalPartPath)) {
+			try {
+				# broken/uncompleted uploaded files shall not go into trash-bin
+				if (class_exists(\OCA\Files_Trashbin\Storage::class)) {
+					\OCA\Files_Trashbin\Storage::$disableTrash = true;
+				}
+				# delete file from storage
+				$partStorage->unlink($internalPartPath);
+				# delete file from cache
+				$partStorage->getCache()->remove($internalPartPath);
+
+			} finally {
+				if (class_exists(\OCA\Files_Trashbin\Storage::class)) {
+					\OCA\Files_Trashbin\Storage::$disableTrash = false;
+				}
+			}
+		}
+	}
 }

apps/dav/tests/unit/Upload/FailedUploadTest.php

@@ -0,0 +1,73 @@
+<?php
+/**
+ * @author Thomas Müller <thomas.mueller@tmit.eu>
+ *
+ * @copyright Copyright (c) 2023, ownCloud GmbH
+ * @license AGPL-3.0
+ *
+ * This code is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License, version 3,
+ * as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License, version 3,
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>
+ *
+ */
+namespace OCA\DAV\Tests\unit\Upload;
+
+use OC\Files\FileInfo;
+use OC\Files\View;
+use OCA\DAV\Connector\Sabre\File;
+use OCP\Lock\ILockingProvider;
+use Sabre\DAV\Exception\BadRequest;
+use Test\TestCase;
+use Test\Traits\UserTrait;
+
+/**
+ * @group DB
+ */
+class FailedUploadTest extends TestCase {
+	use UserTrait;
+
+	public function test(): void {
+		# init
+		$user = $this->createUser('user');
+		$folder = \OC::$server->getUserFolder($user->getUID());
+
+		# fake request
+		$_SERVER['CONTENT_LENGTH'] = 12;
+		$_SERVER['REQUEST_METHOD'] = 'PUT';
+		unset($_SERVER['HTTP_OC_CHUNKED']);
+
+		# perform the request
+		$path = '/test.txt';
+		$info = new FileInfo("user/files/$path", null, null, [], null);
+		$view = new View();
+		$file = new File($view, $info);
+		$file->acquireLock(ILockingProvider::LOCK_SHARED);
+		$stream = fopen('data://text/plain,' . '123456', 'rb');
+		try {
+			$file->put($stream);
+		} catch (BadRequest $e) {
+			self::assertEquals('expected filesize 12 got 6', $e->getMessage());
+		}
+
+		# assert file does not exist
+		self::assertFalse($folder->nodeExists($path));
+		# ensure folder can ge listed
+		$children = $folder->getDirectoryListing();
+		self::assertCount(0, $children);
+
+		# assert there is no file on disk
+		$internalPath = $folder->getInternalPath();
+		self::assertFalse($folder->getStorage()->file_exists($internalPath.'/test.txt'));
+
+		# assert file is not in cache
+		self::assertFalse($folder->getStorage()->getCache()->inCache($internalPath.'/test.txt'));
+	}
+}

apps/files/js/file-upload.js

@@ -1291,8 +1291,9 @@ OC.Uploader.prototype = _.extend({
 						var message = '';
 						if (upload) {
 							var response = upload.getResponse();
-							message = response.message;
+							message = t('files', 'Failed to upload the file "{fileName}": {error}', {fileName: upload.getFileName(), error: response.message});
 						}
+
 						OC.Notification.show(message || data.errorThrown, {type: 'error'});
 					}
 

apps/files_sharing/lib/Controller/Share20OcsController.php

@@ -281,6 +281,7 @@ protected function formatShare(IShare $share, $received = false) {
 	 * Get a specific share by id
 	 *
 	 * @NoAdminRequired
+	 * @NoCSRFRequired
 	 *
 	 * @param string $id
 	 * @return Result
@@ -664,6 +665,7 @@ private function getSharedWithMe($node = null, $includeTags, $requestedShareType
 	 * the function will return an empty list.
 	 *
 	 * @NoAdminRequired
+	 * @NoCSRFRequired
 	 *
 	 * - Get shares by the current user
 	 * - Get shares by the current user and reshares (?reshares=true)

apps/files_trashbin/lib/Storage.php

@@ -39,9 +39,12 @@ class Storage extends Wrapper {
 	/**
 	 * Disable trash logic
 	 *
+	 * NOTE: this public for a very specific purpose to handle broken uploads.
+	 * Don't touch this property unless you know what this is doing! :dancers:
+	 *
 	 * @var bool
 	 */
-	private static $disableTrash = false;
+	public static $disableTrash = false;
 
 	/** @var  IUserManager */
 	private $userManager;