Merge pull request #50270 from nextcloud/fix/share-api-create--permissions

fix(files_sharing): Respect permissions passed when creating link shares

7 files changed, 349 insertions, 253 deletions

diff --git a/apps/files_sharing/lib/Controller/ShareAPIController.php b/apps/files_sharing/lib/Controller/ShareAPIController.php
index e7b4a8d6bb8..1fb62480049 100644
--- a/apps/files_sharing/lib/Controller/ShareAPIController.php
+++ b/apps/files_sharing/lib/Controller/ShareAPIController.php
@@ -10,7 +10,6 @@ declare(strict_types=1);
 namespace OCA\Files_Sharing\Controller;
 
 use Exception;
-use OC\Files\FileInfo;
 use OC\Files\Storage\Wrapper\Wrapper;
 use OCA\Circles\Api\v1\Circles;
 use OCA\Files\Helper;
@@ -536,8 +535,8 @@ class ShareAPIController extends OCSController {
 	 * @param string|null $path Path of the share
 	 * @param int|null $permissions Permissions for the share
 	 * @param int $shareType Type of the share
-	 * @param string|null $shareWith The entity this should be shared with
-	 * @param string $publicUpload If public uploading is allowed
+	 * @param ?string $shareWith The entity this should be shared with
+	 * @param 'true'|'false'|null $publicUpload If public uploading is allowed (deprecated)
 	 * @param string $password Password for the share
 	 * @param string|null $sendPasswordByTalk Send the password for the share over Talk
 	 * @param ?string $expireDate The expiry date of the share in the user's timezone at 00:00.
@@ -562,7 +561,7 @@ class ShareAPIController extends OCSController {
 		?int $permissions = null,
 		int $shareType = -1,
 		?string $shareWith = null,
-		string $publicUpload = 'false',
+		?string $publicUpload = null,
 		string $password = '',
 		?string $sendPasswordByTalk = null,
 		?string $expireDate = null,
@@ -571,18 +570,10 @@ class ShareAPIController extends OCSController {
 		?string $attributes = null,
 		?string $sendMail = null,
 	): DataResponse {
-		$share = $this->shareManager->newShare();
-
-		if ($permissions === null) {
-			if ($shareType === IShare::TYPE_LINK
-				|| $shareType === IShare::TYPE_EMAIL) {
+		assert($this->userId !== null);
 
-				// to keep legacy default behaviour, we ignore the setting below for link shares
-				$permissions = Constants::PERMISSION_READ;
-			} else {
-				$permissions = (int)$this->config->getAppValue('core', 'shareapi_default_permissions', (string)Constants::PERMISSION_ALL);
-			}
-		}
+		$share = $this->shareManager->newShare();
+		$hasPublicUpload = $this->getLegacyPublicUpload($publicUpload);
 
 		// Verify path
 		if ($path === null) {
@@ -601,7 +592,7 @@ class ShareAPIController extends OCSController {
 		// combine all permissions to determine if the user can share this file
 		$nodes = $userFolder->getById($node->getId());
 		foreach ($nodes as $nodeById) {
-			/** @var FileInfo $fileInfo */
+			/** @var \OC\Files\FileInfo $fileInfo */
 			$fileInfo = $node->getFileInfo();
 			$fileInfo['permissions'] |= $nodeById->getPermissions();
 		}
@@ -614,19 +605,23 @@ class ShareAPIController extends OCSController {
 			throw new OCSNotFoundException($this->l->t('Could not create share'));
 		}
 
-		if ($permissions < 0 || $permissions > Constants::PERMISSION_ALL) {
-			throw new OCSNotFoundException($this->l->t('Invalid permissions'));
-		}
-
-		// Shares always require read permissions OR create permissions
-		if (($permissions & Constants::PERMISSION_READ) === 0 && ($permissions & Constants::PERMISSION_CREATE) === 0) {
+		// Set permissions
+		if ($shareType === IShare::TYPE_LINK || $shareType === IShare::TYPE_EMAIL) {
+			$permissions = $this->getLinkSharePermissions($permissions, $hasPublicUpload);
+			$this->validateLinkSharePermissions($node, $permissions, $hasPublicUpload);
+		} else {
+			// Use default permissions only for non-link shares to keep legacy behavior
+			if ($permissions === null) {
+				$permissions = (int)$this->config->getAppValue('core', 'shareapi_default_permissions', (string)Constants::PERMISSION_ALL);
+			}
+			// Non-link shares always require read permissions (link shares could be file drop)
 			$permissions |= Constants::PERMISSION_READ;
 		}
 
+		// For legacy reasons the API allows to pass PERMISSIONS_ALL even for single file shares (I look at you Talk)
 		if ($node instanceof File) {
-			// Single file shares should never have delete or create permissions
-			$permissions &= ~Constants::PERMISSION_DELETE;
-			$permissions &= ~Constants::PERMISSION_CREATE;
+			// if this is a single file share we remove the DELETE and CREATE permissions
+			$permissions = $permissions & ~(Constants::PERMISSION_DELETE | Constants::PERMISSION_CREATE);
 		}
 
 		/**
@@ -701,28 +696,7 @@ class ShareAPIController extends OCSController {
 				throw new OCSNotFoundException($this->l->t('Public link sharing is disabled by the administrator'));
 			}
 
-			if ($publicUpload === 'true') {
-				// Check if public upload is allowed
-				if (!$this->shareManager->shareApiLinkAllowPublicUpload()) {
-					throw new OCSForbiddenException($this->l->t('Public upload disabled by the administrator'));
-				}
-
-				// Public upload can only be set for folders
-				if ($node instanceof File) {
-					throw new OCSNotFoundException($this->l->t('Public upload is only possible for publicly shared folders'));
-				}
-
-				$permissions = Constants::PERMISSION_READ |
-					Constants::PERMISSION_CREATE |
-					Constants::PERMISSION_UPDATE |
-					Constants::PERMISSION_DELETE;
-			}
-
-			// TODO: It might make sense to have a dedicated setting to allow/deny converting link shares into federated ones
-			if ($this->shareManager->outgoingServer2ServerSharesAllowed()) {
-				$permissions |= Constants::PERMISSION_SHARE;
-			}
-
+			$this->validateLinkSharePermissions($node, $permissions, $hasPublicUpload);
 			$share->setPermissions($permissions);
 
 			// Set password
@@ -974,6 +948,71 @@ class ShareAPIController extends OCSController {
 		return new DataResponse($shares);
 	}
 
+	private function getLinkSharePermissions(?int $permissions, ?bool $legacyPublicUpload): int {
+		$permissions = $permissions ?? Constants::PERMISSION_READ;
+
+		// Legacy option handling
+		if ($legacyPublicUpload !== null) {
+			$permissions = $legacyPublicUpload
+				? (Constants::PERMISSION_READ | Constants::PERMISSION_CREATE | Constants::PERMISSION_UPDATE | Constants::PERMISSION_DELETE)
+				: Constants::PERMISSION_READ;
+		}
+
+		// TODO: It might make sense to have a dedicated setting to allow/deny converting link shares into federated ones
+		if ($this->hasPermission($permissions, Constants::PERMISSION_READ)
+			&& $this->shareManager->outgoingServer2ServerSharesAllowed()) {
+			$permissions |= Constants::PERMISSION_SHARE;
+		}
+
+		return $permissions;
+	}
+
+	/**
+	 * Helper to check for legacy "publicUpload" handling.
+	 * If the value is set to `true` or `false` then true or false are returned.
+	 * Otherwise null is returned to indicate that the option was not (or wrong) set.
+	 *
+	 * @param null|string $legacyPublicUpload The value of `publicUpload`
+	 */
+	private function getLegacyPublicUpload(?string $legacyPublicUpload): ?bool {
+		if ($legacyPublicUpload === 'true') {
+			return true;
+		} elseif ($legacyPublicUpload === 'false') {
+			return false;
+		}
+		// Not set at all
+		return null;
+	}
+
+	/**
+	 * For link and email shares validate that only allowed combinations are set.
+	 *
+	 * @throw OCSBadRequestException If permission combination is invalid.
+	 * @throw OCSForbiddenException If public upload was forbidden by the administrator.
+	 */
+	private function validateLinkSharePermissions(Node $node, int $permissions, ?bool $legacyPublicUpload): void {
+		if ($legacyPublicUpload && ($node instanceof File)) {
+			throw new OCSBadRequestException($this->l->t('Public upload is only possible for publicly shared folders'));
+		}
+
+		// We need at least READ or CREATE (file drop)
+		if (!$this->hasPermission($permissions, Constants::PERMISSION_READ)
+			&& !$this->hasPermission($permissions, Constants::PERMISSION_CREATE)) {
+			throw new OCSBadRequestException($this->l->t('Share must at least have READ or CREATE permissions'));
+		}
+
+		// UPDATE and DELETE require a READ permission
+		if (!$this->hasPermission($permissions, Constants::PERMISSION_READ)
+			&& ($this->hasPermission($permissions, Constants::PERMISSION_UPDATE) || $this->hasPermission($permissions, Constants::PERMISSION_DELETE))) {
+			throw new OCSBadRequestException($this->l->t('Share must have READ permission if UPDATE or DELETE permission is set'));
+		}
+
+		// Check if public uploading was disabled
+		if ($this->hasPermission($permissions, Constants::PERMISSION_CREATE)
+			&& !$this->shareManager->shareApiLinkAllowPublicUpload()) {
+			throw new OCSForbiddenException($this->l->t('Public upload disabled by the administrator'));
+		}
+	}
 
 	/**
 	 * @param string $viewer
@@ -1154,7 +1193,6 @@ class ShareAPIController extends OCSController {
 		return ($permissionsSet & $permissionsToCheck) === $permissionsToCheck;
 	}
 
-
 	/**
 	 * Update a share
 	 *
@@ -1242,7 +1280,7 @@ class ShareAPIController extends OCSController {
 		}
 
 		/**
-		 * expirationdate, password and publicUpload only make sense for link shares
+		 * expiration date, password and publicUpload only make sense for link shares
 		 */
 		if ($share->getShareType() === IShare::TYPE_LINK
 			|| $share->getShareType() === IShare::TYPE_EMAIL) {
@@ -1259,58 +1297,13 @@ class ShareAPIController extends OCSController {
 			$share->setAttributes($attributes);
 
 
-			$newPermissions = null;
-			if ($publicUpload === 'true') {
-				$newPermissions = Constants::PERMISSION_READ | Constants::PERMISSION_CREATE | Constants::PERMISSION_UPDATE | Constants::PERMISSION_DELETE;
-			} elseif ($publicUpload === 'false') {
-				$newPermissions = Constants::PERMISSION_READ;
-			}
-
-			if ($permissions !== null) {
-				$newPermissions = $permissions;
-				$newPermissions = $newPermissions & ~Constants::PERMISSION_SHARE;
-			}
-
-			if ($newPermissions !== null) {
-				if (!$this->hasPermission($newPermissions, Constants::PERMISSION_READ) && !$this->hasPermission($newPermissions, Constants::PERMISSION_CREATE)) {
-					throw new OCSBadRequestException($this->l->t('Share must at least have READ or CREATE permissions'));
-				}
-
-				if (!$this->hasPermission($newPermissions, Constants::PERMISSION_READ) && (
-					$this->hasPermission($newPermissions, Constants::PERMISSION_UPDATE) || $this->hasPermission($newPermissions, Constants::PERMISSION_DELETE)
-				)) {
-					throw new OCSBadRequestException($this->l->t('Share must have READ permission if UPDATE or DELETE permission is set'));
-				}
-			}
-
-			if (
-				// legacy
-				$newPermissions === (Constants::PERMISSION_READ | Constants::PERMISSION_CREATE | Constants::PERMISSION_UPDATE) ||
-				// correct
-				$newPermissions === (Constants::PERMISSION_READ | Constants::PERMISSION_CREATE | Constants::PERMISSION_UPDATE | Constants::PERMISSION_DELETE)
-			) {
-				if (!$this->shareManager->shareApiLinkAllowPublicUpload()) {
-					throw new OCSForbiddenException($this->l->t('Public upload disabled by the administrator'));
-				}
-
-				if (!($share->getNode() instanceof Folder)) {
-					throw new OCSBadRequestException($this->l->t('Public upload is only possible for publicly shared folders'));
-				}
-
-				// normalize to correct public upload permissions
-				if ($publicUpload === 'true') {
-					$newPermissions = Constants::PERMISSION_READ | Constants::PERMISSION_CREATE | Constants::PERMISSION_UPDATE | Constants::PERMISSION_DELETE;
-				}
-			}
-
-			if ($newPermissions !== null) {
-				// TODO: It might make sense to have a dedicated setting to allow/deny converting link shares into federated ones
-				if (($newPermissions & Constants::PERMISSION_READ) && $this->shareManager->outgoingServer2ServerSharesAllowed()) {
-					$newPermissions |= Constants::PERMISSION_SHARE;
-				}
-
-				$share->setPermissions($newPermissions);
-				$permissions = $newPermissions;
+			// If either manual permissions are specified or publicUpload
+			// then we need to also update the permissions of the share
+			if ($permissions !== null || $publicUpload !== null) {
+				$hasPublicUpload = $this->getLegacyPublicUpload($publicUpload);
+				$permissions = $this->getLinkSharePermissions($permissions ?? Constants::PERMISSION_READ, $hasPublicUpload);
+				$this->validateLinkSharePermissions($share->getNode(), $permissions, $hasPublicUpload);
+				$share->setPermissions($permissions);
 			}
 
 			if ($password === '') {
diff --git a/apps/files_sharing/openapi.json b/apps/files_sharing/openapi.json
index 145847b9550..e70764a59f2 100644
--- a/apps/files_sharing/openapi.json
+++ b/apps/files_sharing/openapi.json
@@ -1780,8 +1780,12 @@
                                     },
                                     "publicUpload": {
                                         "type": "string",
-                                        "default": "false",
-                                        "description": "If public uploading is allowed"
+                                        "nullable": true,
+                                        "enum": [
+                                            "true",
+                                            "false"
+                                        ],
+                                        "description": "If public uploading is allowed (deprecated)"
                                     },
                                     "password": {
                                         "type": "string",
diff --git a/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php b/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php
index abb98826ab5..f8c316cd2e9 100644
--- a/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php
+++ b/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php
@@ -41,6 +41,7 @@ use OCP\Share\IManager;
 use OCP\Share\IProviderFactory;
 use OCP\Share\IShare;
 use OCP\UserStatus\IManager as IUserStatusManager;
+use PHPUnit\Framework\MockObject\MockObject;
 use Psr\Container\ContainerInterface;
 use Psr\Log\LoggerInterface;
 use Test\TestCase;
@@ -54,24 +55,26 @@ use Test\TestCase;
 class ShareAPIControllerTest extends TestCase {
 
 	private string $appName = 'files_sharing';
-	private \OC\Share20\Manager|\PHPUnit\Framework\MockObject\MockObject $shareManager;
-	private IGroupManager|\PHPUnit\Framework\MockObject\MockObject $groupManager;
-	private IUserManager|\PHPUnit\Framework\MockObject\MockObject $userManager;
-	private IRequest|\PHPUnit\Framework\MockObject\MockObject $request;
-	private IRootFolder|\PHPUnit\Framework\MockObject\MockObject $rootFolder;
-	private IURLGenerator|\PHPUnit\Framework\MockObject\MockObject $urlGenerator;
-	private string|\PHPUnit\Framework\MockObject\MockObject $currentUser;
+	private string $currentUser;
+
 	private ShareAPIController $ocs;
-	private IL10N|\PHPUnit\Framework\MockObject\MockObject $l;
-	private IConfig|\PHPUnit\Framework\MockObject\MockObject $config;
-	private IAppManager|\PHPUnit\Framework\MockObject\MockObject $appManager;
-	private IServerContainer|\PHPUnit\Framework\MockObject\MockObject $serverContainer;
-	private IUserStatusManager|\PHPUnit\Framework\MockObject\MockObject $userStatusManager;
-	private IPreview|\PHPUnit\Framework\MockObject\MockObject $previewManager;
-	private IDateTimeZone|\PHPUnit\Framework\MockObject\MockObject $dateTimeZone;
-	private LoggerInterface $logger;
-	private IProviderFactory|\PHPUnit\Framework\MockObject\MockObject $factory;
-	private IMailer|\PHPUnit\Framework\MockObject\MockObject $mailer;
+
+	private IManager&MockObject $shareManager;
+	private IGroupManager&MockObject $groupManager;
+	private IUserManager&MockObject $userManager;
+	private IRequest&MockObject $request;
+	private IRootFolder&MockObject $rootFolder;
+	private IURLGenerator&MockObject $urlGenerator;
+	private IL10N&MockObject $l;
+	private IConfig&MockObject $config;
+	private IAppManager&MockObject $appManager;
+	private ContainerInterface&MockObject $serverContainer;
+	private IUserStatusManager&MockObject $userStatusManager;
+	private IPreview&MockObject $previewManager;
+	private IDateTimeZone&MockObject $dateTimeZone;
+	private LoggerInterface&MockObject $logger;
+	private IProviderFactory&MockObject $factory;
+	private IMailer&MockObject $mailer;
 
 	protected function setUp(): void {
 		$this->shareManager = $this->createMock(IManager::class);
@@ -131,7 +134,7 @@ class ShareAPIControllerTest extends TestCase {
 	}
 
 	/**
-	 * @return ShareAPIController|\PHPUnit\Framework\MockObject\MockObject
+	 * @return ShareAPIController&MockObject
 	 */
 	private function mockFormatShare() {
 		return $this->getMockBuilder(ShareAPIController::class)
@@ -159,7 +162,7 @@ class ShareAPIControllerTest extends TestCase {
 	}
 
 	private function newShare() {
-		return \OC::$server->getShareManager()->newShare();
+		return \OCP\Server::get(IManager::class)->newShare();
 	}
 
 
@@ -816,7 +819,7 @@ class ShareAPIControllerTest extends TestCase {
 	 * @dataProvider dataGetShare
 	 */
 	public function testGetShare(IShare $share, array $result): void {
-		/** @var ShareAPIController|\PHPUnit\Framework\MockObject\MockObject $ocs */
+		/** @var ShareAPIController&MockObject $ocs */
 		$ocs = $this->getMockBuilder(ShareAPIController::class)
 			->setConstructorArgs([
 				$this->appName,
@@ -908,7 +911,7 @@ class ShareAPIControllerTest extends TestCase {
 		$this->expectException(OCSNotFoundException::class);
 		$this->expectExceptionMessage('Wrong share ID, share does not exist');
 
-		$share = \OC::$server->getShareManager()->newShare();
+		$share = \OCP\Server::get(IManager::class)->newShare();
 		$share->setSharedBy('initiator')
 			->setSharedWith('recipient')
 			->setShareOwner('owner');
@@ -939,7 +942,7 @@ class ShareAPIControllerTest extends TestCase {
 		$folder->method('getDirectoryListing')
 			->willReturn([$file1, $file2]);
 
-		$file1UserShareOwner = \OC::$server->getShareManager()->newShare();
+		$file1UserShareOwner = \OCP\Server::get(IManager::class)->newShare();
 		$file1UserShareOwner->setShareType(IShare::TYPE_USER)
 			->setSharedWith('recipient')
 			->setSharedBy('initiator')
@@ -953,7 +956,7 @@ class ShareAPIControllerTest extends TestCase {
 			'share_type' => IShare::TYPE_USER,
 		];
 
-		$file1UserShareInitiator = \OC::$server->getShareManager()->newShare();
+		$file1UserShareInitiator = \OCP\Server::get(IManager::class)->newShare();
 		$file1UserShareInitiator->setShareType(IShare::TYPE_USER)
 			->setSharedWith('recipient')
 			->setSharedBy('currentUser')
@@ -967,7 +970,7 @@ class ShareAPIControllerTest extends TestCase {
 			'share_type' => IShare::TYPE_USER,
 		];
 
-		$file1UserShareRecipient = \OC::$server->getShareManager()->newShare();
+		$file1UserShareRecipient = \OCP\Server::get(IManager::class)->newShare();
 		$file1UserShareRecipient->setShareType(IShare::TYPE_USER)
 			->setSharedWith('currentUser')
 			->setSharedBy('initiator')
@@ -981,7 +984,7 @@ class ShareAPIControllerTest extends TestCase {
 			'share_type' => IShare::TYPE_USER,
 		];
 
-		$file1UserShareOther = \OC::$server->getShareManager()->newShare();
+		$file1UserShareOther = \OCP\Server::get(IManager::class)->newShare();
 		$file1UserShareOther->setShareType(IShare::TYPE_USER)
 			->setSharedWith('recipient')
 			->setSharedBy('initiator')
@@ -995,7 +998,7 @@ class ShareAPIControllerTest extends TestCase {
 			'share_type' => IShare::TYPE_USER,
 		];
 
-		$file1GroupShareOwner = \OC::$server->getShareManager()->newShare();
+		$file1GroupShareOwner = \OCP\Server::get(IManager::class)->newShare();
 		$file1GroupShareOwner->setShareType(IShare::TYPE_GROUP)
 			->setSharedWith('recipient')
 			->setSharedBy('initiator')
@@ -1009,7 +1012,7 @@ class ShareAPIControllerTest extends TestCase {
 			'share_type' => IShare::TYPE_GROUP,
 		];
 
-		$file1GroupShareRecipient = \OC::$server->getShareManager()->newShare();
+		$file1GroupShareRecipient = \OCP\Server::get(IManager::class)->newShare();
 		$file1GroupShareRecipient->setShareType(IShare::TYPE_GROUP)
 			->setSharedWith('currentUserGroup')
 			->setSharedBy('initiator')
@@ -1023,7 +1026,7 @@ class ShareAPIControllerTest extends TestCase {
 			'share_type' => IShare::TYPE_GROUP,
 		];
 
-		$file1GroupShareOther = \OC::$server->getShareManager()->newShare();
+		$file1GroupShareOther = \OCP\Server::get(IManager::class)->newShare();
 		$file1GroupShareOther->setShareType(IShare::TYPE_GROUP)
 			->setSharedWith('recipient')
 			->setSharedBy('initiator')
@@ -1032,7 +1035,7 @@ class ShareAPIControllerTest extends TestCase {
 			->setNode($file1)
 			->setId(108);
 
-		$file1LinkShareOwner = \OC::$server->getShareManager()->newShare();
+		$file1LinkShareOwner = \OCP\Server::get(IManager::class)->newShare();
 		$file1LinkShareOwner->setShareType(IShare::TYPE_LINK)
 			->setSharedWith('recipient')
 			->setSharedBy('initiator')
@@ -1046,7 +1049,7 @@ class ShareAPIControllerTest extends TestCase {
 			'share_type' => IShare::TYPE_LINK,
 		];
 
-		$file1EmailShareOwner = \OC::$server->getShareManager()->newShare();
+		$file1EmailShareOwner = \OCP\Server::get(IManager::class)->newShare();
 		$file1EmailShareOwner->setShareType(IShare::TYPE_EMAIL)
 			->setSharedWith('recipient')
 			->setSharedBy('initiator')
@@ -1060,7 +1063,7 @@ class ShareAPIControllerTest extends TestCase {
 			'share_type' => IShare::TYPE_EMAIL,
 		];
 
-		$file1CircleShareOwner = \OC::$server->getShareManager()->newShare();
+		$file1CircleShareOwner = \OCP\Server::get(IManager::class)->newShare();
 		$file1CircleShareOwner->setShareType(IShare::TYPE_CIRCLE)
 			->setSharedWith('recipient')
 			->setSharedBy('initiator')
@@ -1074,7 +1077,7 @@ class ShareAPIControllerTest extends TestCase {
 			'share_type' => IShare::TYPE_CIRCLE,
 		];
 
-		$file1RoomShareOwner = \OC::$server->getShareManager()->newShare();
+		$file1RoomShareOwner = \OCP\Server::get(IManager::class)->newShare();
 		$file1RoomShareOwner->setShareType(IShare::TYPE_ROOM)
 			->setSharedWith('recipient')
 			->setSharedBy('initiator')
@@ -1088,7 +1091,7 @@ class ShareAPIControllerTest extends TestCase {
 			'share_type' => IShare::TYPE_ROOM,
 		];
 
-		$file1RemoteShareOwner = \OC::$server->getShareManager()->newShare();
+		$file1RemoteShareOwner = \OCP\Server::get(IManager::class)->newShare();
 		$file1RemoteShareOwner->setShareType(IShare::TYPE_REMOTE)
 			->setSharedWith('recipient')
 			->setSharedBy('initiator')
@@ -1103,7 +1106,7 @@ class ShareAPIControllerTest extends TestCase {
 			'share_type' => IShare::TYPE_REMOTE,
 		];
 
-		$file1RemoteGroupShareOwner = \OC::$server->getShareManager()->newShare();
+		$file1RemoteGroupShareOwner = \OCP\Server::get(IManager::class)->newShare();
 		$file1RemoteGroupShareOwner->setShareType(IShare::TYPE_REMOTE_GROUP)
 			->setSharedWith('recipient')
 			->setSharedBy('initiator')
@@ -1118,7 +1121,7 @@ class ShareAPIControllerTest extends TestCase {
 			'share_type' => IShare::TYPE_REMOTE_GROUP,
 		];
 
-		$file2UserShareOwner = \OC::$server->getShareManager()->newShare();
+		$file2UserShareOwner = \OCP\Server::get(IManager::class)->newShare();
 		$file2UserShareOwner->setShareType(IShare::TYPE_USER)
 			->setSharedWith('recipient')
 			->setSharedBy('initiator')
@@ -1450,7 +1453,7 @@ class ShareAPIControllerTest extends TestCase {
 	 * @dataProvider dataGetShares
 	 */
 	public function testGetShares(array $getSharesParameters, array $shares, array $extraShareTypes, array $expected): void {
-		/** @var ShareAPIController $ocs */
+		/** @var ShareAPIController&MockObject $ocs */
 		$ocs = $this->getMockBuilder(ShareAPIController::class)
 			->setConstructorArgs([
 				$this->appName,
@@ -1471,7 +1474,8 @@ class ShareAPIControllerTest extends TestCase {
 				$this->factory,
 				$this->mailer,
 				$this->currentUser,
-			])->setMethods(['formatShare'])
+			])
+			->onlyMethods(['formatShare'])
 			->getMock();
 
 		$ocs->method('formatShare')
@@ -1702,36 +1706,33 @@ class ShareAPIControllerTest extends TestCase {
 		$this->ocs->createShare('invalid-path');
 	}
 
-
-	public function testCreateShareInvalidPermissions(): void {
-		$this->expectException(OCSNotFoundException::class);
-		$this->expectExceptionMessage('Invalid permissions');
+	public function testCreateShareInvalidShareType(): void {
+		$this->expectException(OCSBadRequestException::class);
+		$this->expectExceptionMessage('Unknown share type');
 
 		$share = $this->newShare();
 		$this->shareManager->method('newShare')->willReturn($share);
 
-		$userFolder = $this->getMockBuilder(Folder::class)->getMock();
-		$this->rootFolder->expects($this->once())
+		[$userFolder, $file] = $this->getNonSharedUserFile();
+		$this->rootFolder->expects($this->atLeastOnce())
 			->method('getUserFolder')
 			->with('currentUser')
 			->willReturn($userFolder);
 
-		$path = $this->getMockBuilder(File::class)->getMock();
-		$userFolder->expects($this->once())
+		$userFolder->expects($this->atLeastOnce())
 			->method('get')
 			->with('valid-path')
-			->willReturn($path);
+			->willReturn($file);
 		$userFolder->method('getById')
 			->willReturn([]);
 
-		$path->expects($this->once())
+		$file->expects($this->once())
 			->method('lock')
 			->with(ILockingProvider::LOCK_SHARED);
 
-		$this->ocs->createShare('valid-path', 32);
+		$this->ocs->createShare('valid-path', 31);
 	}
 
-
 	public function testCreateShareUserNoShareWith(): void {
 		$this->expectException(OCSNotFoundException::class);
 		$this->expectExceptionMessage('Please specify a valid account to share with');
@@ -1891,7 +1892,7 @@ class ShareAPIControllerTest extends TestCase {
 		$share = $this->newShare();
 		$this->shareManager->method('newShare')->willReturn($share);
 
-		/** @var ShareAPIController|\PHPUnit\Framework\MockObject\MockObject $ocs */
+		/** @var ShareAPIController&MockObject $ocs */
 		$ocs = $this->getMockBuilder(ShareAPIController::class)
 			->setConstructorArgs([
 				$this->appName,
@@ -2020,7 +2021,9 @@ class ShareAPIControllerTest extends TestCase {
 		$this->rootFolder->method('getById')
 			->willReturn([]);
 
-		$this->shareManager->method('newShare')->willReturn(\OC::$server->getShareManager()->newShare());
+		$this->shareManager->method('newShare')->willReturn(\OCP\Server::get(IManager::class)->newShare());
+		$this->shareManager->method('shareApiLinkAllowPublicUpload')->willReturn(true);
+		$this->shareManager->method('shareApiAllowLinks')->willReturn(false);
 
 		$this->ocs->createShare('valid-path', Constants::PERMISSION_ALL, IShare::TYPE_LINK);
 	}
@@ -2044,7 +2047,7 @@ class ShareAPIControllerTest extends TestCase {
 		$this->rootFolder->method('getById')
 			->willReturn([]);
 
-		$this->shareManager->method('newShare')->willReturn(\OC::$server->getShareManager()->newShare());
+		$this->shareManager->method('newShare')->willReturn(\OCP\Server::get(IManager::class)->newShare());
 		$this->shareManager->method('shareApiAllowLinks')->willReturn(true);
 
 		$this->ocs->createShare('valid-path', Constants::PERMISSION_ALL, IShare::TYPE_LINK, null, 'true');
@@ -2052,24 +2055,26 @@ class ShareAPIControllerTest extends TestCase {
 
 
 	public function testCreateShareLinkPublicUploadFile(): void {
-		$this->expectException(OCSNotFoundException::class);
+		$this->expectException(OCSBadRequestException::class);
 		$this->expectExceptionMessage('Public upload is only possible for publicly shared folders');
 
-		$path = $this->getMockBuilder(File::class)->getMock();
-		$path->method('getId')->willReturn(42);
 		$storage = $this->createMock(IStorage::class);
 		$storage->method('instanceOfStorage')
 			->willReturnMap([
 				['OCA\Files_Sharing\External\Storage', false],
 				['OCA\Files_Sharing\SharedStorage', false],
 			]);
-		$path->method('getStorage')->willReturn($storage);
+
+		$file = $this->createMock(File::class);
+		$file->method('getId')->willReturn(42);
+		$file->method('getStorage')->willReturn($storage);
+	
 		$this->rootFolder->method('getUserFolder')->with($this->currentUser)->willReturnSelf();
-		$this->rootFolder->method('get')->with('valid-path')->willReturn($path);
+		$this->rootFolder->method('get')->with('valid-path')->willReturn($file);
 		$this->rootFolder->method('getById')
 			->willReturn([]);
 
-		$this->shareManager->method('newShare')->willReturn(\OC::$server->getShareManager()->newShare());
+		$this->shareManager->method('newShare')->willReturn(\OCP\Server::get(IManager::class)->newShare());
 		$this->shareManager->method('shareApiAllowLinks')->willReturn(true);
 		$this->shareManager->method('shareApiLinkAllowPublicUpload')->willReturn(true);
 
@@ -2093,7 +2098,7 @@ class ShareAPIControllerTest extends TestCase {
 		$this->rootFolder->method('getById')
 			->willReturn([]);
 
-		$this->shareManager->method('newShare')->willReturn(\OC::$server->getShareManager()->newShare());
+		$this->shareManager->method('newShare')->willReturn(\OCP\Server::get(IManager::class)->newShare());
 		$this->shareManager->method('shareApiAllowLinks')->willReturn(true);
 		$this->shareManager->method('shareApiLinkAllowPublicUpload')->willReturn(true);
 
@@ -2132,23 +2137,23 @@ class ShareAPIControllerTest extends TestCase {
 		$this->rootFolder->method('getById')
 			->willReturn([]);
 
-		$this->shareManager->method('newShare')->willReturn(\OC::$server->getShareManager()->newShare());
+		$this->shareManager->method('newShare')->willReturn(\OCP\Server::get(IManager::class)->newShare());
 		$this->shareManager->method('shareApiAllowLinks')->willReturn(true);
 		$this->shareManager->method('shareApiLinkAllowPublicUpload')->willReturn(true);
 
 		$this->shareManager->expects($this->once())->method('createShare')->with(
 			$this->callback(function (IShare $share) use ($path) {
-				return $share->getNode() === $path &&
-				$share->getShareType() === IShare::TYPE_LINK &&
-				$share->getPermissions() === Constants::PERMISSION_ALL &&
-				$share->getSharedBy() === 'currentUser' &&
-				$share->getPassword() === 'password' &&
-				$share->getExpirationDate() === null;
+				return $share->getNode() === $path
+				&& $share->getShareType() === IShare::TYPE_LINK
+				&& $share->getPermissions() === Constants::PERMISSION_READ // publicUpload was set to false
+				&& $share->getSharedBy() === 'currentUser'
+				&& $share->getPassword() === 'password'
+				&& $share->getExpirationDate() === null;
 			})
 		)->willReturnArgument(0);
 
 		$expected = new DataResponse([]);
-		$result = $ocs->createShare('valid-path', Constants::PERMISSION_ALL, IShare::TYPE_LINK, null, 'false', 'password', null, '');
+		$result = $ocs->createShare('valid-path', Constants::PERMISSION_READ, IShare::TYPE_LINK, null, 'false', 'password', null, '');
 
 		$this->assertInstanceOf(get_class($expected), $result);
 		$this->assertEquals($expected->getData(), $result->getData());
@@ -2171,7 +2176,7 @@ class ShareAPIControllerTest extends TestCase {
 		$this->rootFolder->method('getById')
 			->willReturn([]);
 
-		$this->shareManager->method('newShare')->willReturn(\OC::$server->getShareManager()->newShare());
+		$this->shareManager->method('newShare')->willReturn(\OCP\Server::get(IManager::class)->newShare());
 		$this->shareManager->method('shareApiAllowLinks')->willReturn(true);
 		$this->shareManager->method('shareApiLinkAllowPublicUpload')->willReturn(true);
 
@@ -2181,7 +2186,7 @@ class ShareAPIControllerTest extends TestCase {
 			$this->callback(function (IShare $share) use ($path) {
 				return $share->getNode() === $path &&
 				$share->getShareType() === IShare::TYPE_LINK &&
-				$share->getPermissions() === Constants::PERMISSION_ALL &&
+				$share->getPermissions() === (Constants::PERMISSION_ALL & ~(Constants::PERMISSION_SHARE)) &&
 				$share->getSharedBy() === 'currentUser' &&
 				$share->getPassword() === 'password' &&
 				$share->getSendPasswordByTalk() === true &&
@@ -2190,7 +2195,7 @@ class ShareAPIControllerTest extends TestCase {
 		)->willReturnArgument(0);
 
 		$expected = new DataResponse([]);
-		$result = $ocs->createShare('valid-path', Constants::PERMISSION_ALL, IShare::TYPE_LINK, null, 'false', 'password', 'true', '');
+		$result = $ocs->createShare('valid-path', Constants::PERMISSION_ALL, IShare::TYPE_LINK, null, 'true', 'password', 'true', '');
 
 		$this->assertInstanceOf(get_class($expected), $result);
 		$this->assertEquals($expected->getData(), $result->getData());
@@ -2218,7 +2223,7 @@ class ShareAPIControllerTest extends TestCase {
 		$this->rootFolder->method('getById')
 			->willReturn([]);
 
-		$this->shareManager->method('newShare')->willReturn(\OC::$server->getShareManager()->newShare());
+		$this->shareManager->method('newShare')->willReturn(\OCP\Server::get(IManager::class)->newShare());
 		$this->shareManager->method('shareApiAllowLinks')->willReturn(true);
 		$this->shareManager->method('shareApiLinkAllowPublicUpload')->willReturn(true);
 
@@ -2256,7 +2261,7 @@ class ShareAPIControllerTest extends TestCase {
 		$this->rootFolder->method('getById')
 			->willReturn([]);
 
-		$this->shareManager->method('newShare')->willReturn(\OC::$server->getShareManager()->newShare());
+		$this->shareManager->method('newShare')->willReturn(\OCP\Server::get(IManager::class)->newShare());
 		$this->shareManager->method('shareApiAllowLinks')->willReturn(true);
 		$this->shareManager->method('shareApiLinkAllowPublicUpload')->willReturn(true);
 
@@ -2302,7 +2307,7 @@ class ShareAPIControllerTest extends TestCase {
 		$this->rootFolder->method('getById')
 			->willReturn([]);
 
-		$this->shareManager->method('newShare')->willReturn(\OC::$server->getShareManager()->newShare());
+		$this->shareManager->method('newShare')->willReturn(\OCP\Server::get(IManager::class)->newShare());
 		$this->shareManager->method('shareApiAllowLinks')->willReturn(true);
 		$this->shareManager->method('shareApiLinkAllowPublicUpload')->willReturn(true);
 
@@ -2490,11 +2495,7 @@ class ShareAPIControllerTest extends TestCase {
 			)->willReturnCallback(
 				function ($share): void {
 					$share->setSharedWith('recipientRoom');
-					$share->setPermissions(
-						Constants::PERMISSION_ALL &
-						~Constants::PERMISSION_DELETE &
-						~Constants::PERMISSION_CREATE
-					);
+					$share->setPermissions(Constants::PERMISSION_ALL);
 				}
 			);
 
@@ -2504,15 +2505,11 @@ class ShareAPIControllerTest extends TestCase {
 
 		$this->shareManager->method('createShare')
 			->with($this->callback(function (IShare $share) use ($path) {
-				return $share->getNode() === $path &&
-					$share->getPermissions() === (
-						Constants::PERMISSION_ALL &
-						~Constants::PERMISSION_DELETE &
-						~Constants::PERMISSION_CREATE
-					) &&
-					$share->getShareType() === IShare::TYPE_ROOM &&
-					$share->getSharedWith() === 'recipientRoom' &&
-					$share->getSharedBy() === 'currentUser';
+				return $share->getNode() === $path
+					&& $share->getPermissions() === Constants::PERMISSION_ALL
+					&& $share->getShareType() === IShare::TYPE_ROOM
+					&& $share->getSharedWith() === 'recipientRoom'
+					&& $share->getSharedBy() === 'currentUser';
 			}))
 			->willReturnArgument(0);
 
@@ -2593,15 +2590,13 @@ class ShareAPIControllerTest extends TestCase {
 			->willReturn(true);
 
 		$helper = $this->getMockBuilder('\OCA\Talk\Share\Helper\ShareAPIController')
-			->setMethods(['createShare'])
+			->addMethods(['createShare'])
 			->getMock();
 		$helper->method('createShare')
 			->with(
 				$share,
 				'recipientRoom',
-				Constants::PERMISSION_ALL &
-				~Constants::PERMISSION_DELETE &
-				~Constants::PERMISSION_CREATE,
+				Constants::PERMISSION_ALL & ~(Constants::PERMISSION_CREATE | Constants::PERMISSION_DELETE),
 				''
 			)->willReturnCallback(
 				function ($share): void {
@@ -2623,10 +2618,10 @@ class ShareAPIControllerTest extends TestCase {
 	 * TODO: Remove once proper solution is in place
 	 */
 	public function testCreateReshareOfFederatedMountNoDeletePermissions(): void {
-		$share = \OC::$server->getShareManager()->newShare();
+		$share = \OCP\Server::get(IManager::class)->newShare();
 		$this->shareManager->method('newShare')->willReturn($share);
 
-		/** @var ShareAPIController|\PHPUnit\Framework\MockObject\MockObject $ocs */
+		/** @var ShareAPIController&MockObject $ocs */
 		$ocs = $this->getMockBuilder(ShareAPIController::class)
 			->setConstructorArgs([
 				$this->appName,
@@ -2825,7 +2820,7 @@ class ShareAPIControllerTest extends TestCase {
 		$folder->method('getId')
 			->willReturn(42);
 
-		$share = \OC::$server->getShareManager()->newShare();
+		$share = \OCP\Server::get(IManager::class)->newShare();
 		$share->setPermissions(Constants::PERMISSION_ALL)
 			->setSharedBy($this->currentUser)
 			->setShareType(IShare::TYPE_LINK)
@@ -2882,7 +2877,7 @@ class ShareAPIControllerTest extends TestCase {
 		$folder->method('getId')
 			->willReturn(42);
 
-		$share = \OC::$server->getShareManager()->newShare();
+		$share = \OCP\Server::get(IManager::class)->newShare();
 		$share->setPermissions(Constants::PERMISSION_ALL)
 			->setSharedBy($this->currentUser)
 			->setShareType(IShare::TYPE_LINK)
@@ -2943,7 +2938,7 @@ class ShareAPIControllerTest extends TestCase {
 		$folder->method('getId')
 			->willReturn(42);
 
-		$share = \OC::$server->getShareManager()->newShare();
+		$share = \OCP\Server::get(IManager::class)->newShare();
 		$share->setPermissions(Constants::PERMISSION_ALL)
 			->setSharedBy($this->currentUser)
 			->setShareType(IShare::TYPE_LINK)
@@ -2974,7 +2969,7 @@ class ShareAPIControllerTest extends TestCase {
 			->willReturn(42);
 
 		$expected = new DataResponse([]);
-		$result = $ocs->updateShare(42, $permissions, 'password', null, 'true', null);
+		$result = $ocs->updateShare(42, $permissions, 'password', null, null, null);
 
 		$this->assertInstanceOf(get_class($expected), $result);
 		$this->assertEquals($expected->getData(), $result->getData());
@@ -2995,7 +2990,7 @@ class ShareAPIControllerTest extends TestCase {
 		$this->expectException(OCSBadRequestException::class);
 		$this->expectExceptionMessage('Share must at least have READ or CREATE permissions');
 
-		$this->testUpdateLinkShareSetCRUDPermissions($permissions);
+		$this->testUpdateLinkShareSetCRUDPermissions($permissions, null);
 	}
 
 	public function publicLinkInvalidPermissionsProvider2() {
@@ -3031,7 +3026,7 @@ class ShareAPIControllerTest extends TestCase {
 		$folder->method('getId')
 			->willReturn(42);
 
-		$share = \OC::$server->getShareManager()->newShare();
+		$share = \OCP\Server::get(IManager::class)->newShare();
 		$share->setPermissions(Constants::PERMISSION_ALL)
 			->setSharedBy($this->currentUser)
 			->setShareType(IShare::TYPE_LINK)
@@ -3077,7 +3072,7 @@ class ShareAPIControllerTest extends TestCase {
 
 		$folder->method('getId')->willReturn(42);
 
-		$share = \OC::$server->getShareManager()->newShare();
+		$share = \OCP\Server::get(IManager::class)->newShare();
 		$share->setPermissions(Constants::PERMISSION_ALL)
 			->setSharedBy($this->currentUser)
 			->setShareType(IShare::TYPE_LINK)
@@ -3096,25 +3091,31 @@ class ShareAPIControllerTest extends TestCase {
 
 		$ocs = $this->mockFormatShare();
 
-		$file = $this->getMockBuilder(File::class)->getMock();
-		$file->method('getId')
-			->willReturn(42);
-		[$userFolder, $folder] = $this->getNonSharedUserFolder();
+		[$userFolder, $file] = $this->getNonSharedUserFile();
 		$userFolder->method('getFirstNodeById')
 			->with(42)
-			->willReturn($folder);
+			->willReturn($file);
 		$this->rootFolder->method('getUserFolder')
 			->with($this->currentUser)
 			->willReturn($userFolder);
 
-		$share = \OC::$server->getShareManager()->newShare();
+		$share = \OCP\Server::get(IManager::class)->newShare();
 		$share->setPermissions(Constants::PERMISSION_ALL)
 			->setSharedBy($this->currentUser)
 			->setShareType(IShare::TYPE_LINK)
 			->setNode($file);
 
-		$this->shareManager->method('getShareById')->with('ocinternal:42')->willReturn($share);
-		$this->shareManager->method('shareApiLinkAllowPublicUpload')->willReturn(true);
+		$this->shareManager
+			->method('getShareById')
+			->with('ocinternal:42')
+			->willReturn($share);
+		$this->shareManager
+			->method('shareApiLinkAllowPublicUpload')
+			->willReturn(true);
+		$this->shareManager
+			->method('updateShare')
+			->with($share)
+			->willThrowException(new \InvalidArgumentException('File shares cannot have create or delete permissions'));
 
 		$ocs->updateShare(42, null, 'password', null, 'true', '');
 	}
@@ -3464,7 +3465,7 @@ class ShareAPIControllerTest extends TestCase {
 		$folder->method('getId')
 			->willReturn(42);
 
-		$share = \OC::$server->getShareManager()->newShare();
+		$share = \OCP\Server::get(IManager::class)->newShare();
 		$share->setPermissions(Constants::PERMISSION_ALL)
 			->setSharedBy($this->currentUser)
 			->setShareType(IShare::TYPE_LINK)
@@ -3525,7 +3526,7 @@ class ShareAPIControllerTest extends TestCase {
 		$folder->method('getId')
 			->willReturn(42);
 
-		$share = \OC::$server->getShareManager()->newShare();
+		$share = \OCP\Server::get(IManager::class)->newShare();
 		$share->setPermissions(Constants::PERMISSION_ALL)
 			->setSharedBy($this->currentUser)
 			->setShareType(IShare::TYPE_LINK)
@@ -3585,7 +3586,7 @@ class ShareAPIControllerTest extends TestCase {
 		$folder->method('getId')
 			->willReturn(42);
 
-		$share = \OC::$server->getShareManager()->newShare();
+		$share = \OCP\Server::get(IManager::class)->newShare();
 		$share->setPermissions(Constants::PERMISSION_ALL)
 			->setSharedBy($this->currentUser)
 			->setShareType(IShare::TYPE_LINK)
@@ -3601,17 +3602,19 @@ class ShareAPIControllerTest extends TestCase {
 		$this->shareManager->method('getShareById')->with('ocinternal:42')->willReturn($share);
 		$this->shareManager->method('shareApiLinkAllowPublicUpload')->willReturn(true);
 
-		$this->shareManager->expects($this->once())->method('updateShare')->with(
-			$this->callback(function (IShare $share) use ($date) {
-				return $share->getPermissions() === (Constants::PERMISSION_READ | Constants::PERMISSION_CREATE | Constants::PERMISSION_UPDATE | Constants::PERMISSION_DELETE) &&
-					$share->getPassword() === 'password' &&
-					$share->getSendPasswordByTalk() === true &&
-					$share->getExpirationDate() === $date &&
-					$share->getNote() === 'note' &&
-					$share->getLabel() === 'label' &&
-					$share->getHideDownload() === true;
-			})
-		)->willReturnArgument(0);
+		$this->shareManager->expects($this->once())
+			->method('updateShare')
+			->with(
+				$this->callback(function (IShare $share) use ($date) {
+					return $share->getPermissions() === Constants::PERMISSION_ALL &&
+						$share->getPassword() === 'password' &&
+						$share->getSendPasswordByTalk() === true &&
+						$share->getExpirationDate() === $date &&
+						$share->getNote() === 'note' &&
+						$share->getLabel() === 'label' &&
+						$share->getHideDownload() === true;
+				})
+			)->willReturnArgument(0);
 
 		$this->rootFolder->method('getUserFolder')
 			->with($this->currentUser)
@@ -3630,7 +3633,7 @@ class ShareAPIControllerTest extends TestCase {
 		$this->shareManager->method('getSharedWith')->willReturn([]);
 
 		$expected = new DataResponse([]);
-		$result = $ocs->updateShare(42, 31, null, null, null, null, null, null, null);
+		$result = $ocs->updateShare(42, Constants::PERMISSION_ALL, null, null, null, null, null, null, null);
 
 		$this->assertInstanceOf(get_class($expected), $result);
 		$this->assertEquals($expected->getData(), $result->getData());
@@ -3643,7 +3646,7 @@ class ShareAPIControllerTest extends TestCase {
 		$file->method('getId')
 			->willReturn(42);
 
-		$share = \OC::$server->getShareManager()->newShare();
+		$share = \OCP\Server::get(IManager::class)->newShare();
 		$share->setPermissions(Constants::PERMISSION_ALL)
 			->setSharedBy($this->currentUser)
 			->setShareType(IShare::TYPE_USER)
@@ -3689,7 +3692,7 @@ class ShareAPIControllerTest extends TestCase {
 		$folder->method('getId')
 			->willReturn(42);
 
-		$share = \OC::$server->getShareManager()->newShare();
+		$share = \OCP\Server::get(IManager::class)->newShare();
 		$share
 			->setId(42)
 			->setSharedBy($this->currentUser)
@@ -3701,7 +3704,7 @@ class ShareAPIControllerTest extends TestCase {
 
 		// note: updateShare will modify the received instance but getSharedWith will reread from the database,
 		// so their values will be different
-		$incomingShare = \OC::$server->getShareManager()->newShare();
+		$incomingShare = \OCP\Server::get(IManager::class)->newShare();
 		$incomingShare
 			->setId(42)
 			->setSharedBy($this->currentUser)
@@ -3761,7 +3764,7 @@ class ShareAPIControllerTest extends TestCase {
 		$folder->method('getId')
 			->willReturn(42);
 
-		$share = \OC::$server->getShareManager()->newShare();
+		$share = \OCP\Server::get(IManager::class)->newShare();
 		$share
 			->setId(42)
 			->setSharedBy($this->currentUser)
@@ -3773,7 +3776,7 @@ class ShareAPIControllerTest extends TestCase {
 
 		// note: updateShare will modify the received instance but getSharedWith will reread from the database,
 		// so their values will be different
-		$incomingShare = \OC::$server->getShareManager()->newShare();
+		$incomingShare = \OCP\Server::get(IManager::class)->newShare();
 		$incomingShare
 			->setId(42)
 			->setSharedBy($this->currentUser)
@@ -3935,7 +3938,7 @@ class ShareAPIControllerTest extends TestCase {
 
 		$result = [];
 
-		$share = \OC::$server->getShareManager()->newShare();
+		$share = \OCP\Server::get(IManager::class)->newShare();
 		$share->setShareType(IShare::TYPE_USER)
 			->setSharedWith('recipient')
 			->setSharedBy('initiator')
@@ -4037,7 +4040,7 @@ class ShareAPIControllerTest extends TestCase {
 			], false
 		];
 
-		$share = \OC::$server->getShareManager()->newShare();
+		$share = \OCP\Server::get(IManager::class)->newShare();
 		$share->setShareType(IShare::TYPE_USER)
 			->setSharedWith('recipient')
 			->setSharedBy('initiator')
@@ -4091,7 +4094,7 @@ class ShareAPIControllerTest extends TestCase {
 			], $share, [], false
 		];
 
-		$share = \OC::$server->getShareManager()->newShare();
+		$share = \OCP\Server::get(IManager::class)->newShare();
 		$share->setShareType(IShare::TYPE_USER)
 			->setSharedWith('recipient')
 			->setSharedBy('initiator')
@@ -4147,7 +4150,7 @@ class ShareAPIControllerTest extends TestCase {
 
 		// with existing group
 
-		$share = \OC::$server->getShareManager()->newShare();
+		$share = \OCP\Server::get(IManager::class)->newShare();
 		$share->setShareType(IShare::TYPE_GROUP)
 			->setSharedWith('recipientGroup')
 			->setSharedBy('initiator')
@@ -4201,7 +4204,7 @@ class ShareAPIControllerTest extends TestCase {
 		];
 
 		// with unknown group / no group backend
-		$share = \OC::$server->getShareManager()->newShare();
+		$share = \OCP\Server::get(IManager::class)->newShare();
 		$share->setShareType(IShare::TYPE_GROUP)
 			->setSharedWith('recipientGroup2')
 			->setSharedBy('initiator')
@@ -4252,7 +4255,7 @@ class ShareAPIControllerTest extends TestCase {
 			], $share, [], false
 		];
 
-		$share = \OC::$server->getShareManager()->newShare();
+		$share = \OCP\Server::get(IManager::class)->newShare();
 		$share->setShareType(IShare::TYPE_LINK)
 			->setSharedBy('initiator')
 			->setShareOwner('owner')
@@ -4311,7 +4314,7 @@ class ShareAPIControllerTest extends TestCase {
 			], $share, [], false
 		];
 
-		$share = \OC::$server->getShareManager()->newShare();
+		$share = \OCP\Server::get(IManager::class)->newShare();
 		$share->setShareType(IShare::TYPE_LINK)
 			->setSharedBy('initiator')
 			->setShareOwner('owner')
@@ -4370,7 +4373,7 @@ class ShareAPIControllerTest extends TestCase {
 			], $share, [], false
 		];
 
-		$share = \OC::$server->getShareManager()->newShare();
+		$share = \OCP\Server::get(IManager::class)->newShare();
 		$share->setShareType(IShare::TYPE_REMOTE)
 			->setSharedBy('initiator')
 			->setSharedWith('user@server.com')
@@ -4423,7 +4426,7 @@ class ShareAPIControllerTest extends TestCase {
 			], $share, [], false
 		];
 
-		$share = \OC::$server->getShareManager()->newShare();
+		$share = \OCP\Server::get(IManager::class)->newShare();
 		$share->setShareType(IShare::TYPE_REMOTE_GROUP)
 			->setSharedBy('initiator')
 			->setSharedWith('user@server.com')
@@ -4477,7 +4480,7 @@ class ShareAPIControllerTest extends TestCase {
 		];
 
 		// Circle with id, display name and avatar set by the Circles app
-		$share = \OC::$server->getShareManager()->newShare();
+		$share = \OCP\Server::get(IManager::class)->newShare();
 		$share->setShareType(IShare::TYPE_CIRCLE)
 			->setSharedBy('initiator')
 			->setSharedWith('Circle (Public circle, circleOwner) [4815162342]')
@@ -4533,7 +4536,7 @@ class ShareAPIControllerTest extends TestCase {
 		];
 
 		// Circle with id set by the Circles app
-		$share = \OC::$server->getShareManager()->newShare();
+		$share = \OCP\Server::get(IManager::class)->newShare();
 		$share->setShareType(IShare::TYPE_CIRCLE)
 			->setSharedBy('initiator')
 			->setSharedWith('Circle (Public circle, circleOwner) [4815162342]')
@@ -4586,7 +4589,7 @@ class ShareAPIControllerTest extends TestCase {
 		];
 
 		// Circle with id not set by the Circles app
-		$share = \OC::$server->getShareManager()->newShare();
+		$share = \OCP\Server::get(IManager::class)->newShare();
 		$share->setShareType(IShare::TYPE_CIRCLE)
 			->setSharedBy('initiator')
 			->setSharedWith('Circle (Public circle, circleOwner)')
@@ -4638,7 +4641,7 @@ class ShareAPIControllerTest extends TestCase {
 			], $share, [], false
 		];
 
-		$share = \OC::$server->getShareManager()->newShare();
+		$share = \OCP\Server::get(IManager::class)->newShare();
 		$share->setShareType(IShare::TYPE_USER)
 			->setSharedBy('initiator')
 			->setSharedWith('recipient')
@@ -4653,7 +4656,7 @@ class ShareAPIControllerTest extends TestCase {
 			[], $share, [], true
 		];
 
-		$share = \OC::$server->getShareManager()->newShare();
+		$share = \OCP\Server::get(IManager::class)->newShare();
 		$share->setShareType(IShare::TYPE_EMAIL)
 			->setSharedBy('initiator')
 			->setSharedWith('user@server.com')
@@ -4708,7 +4711,7 @@ class ShareAPIControllerTest extends TestCase {
 			], $share, [], false
 		];
 
-		$share = \OC::$server->getShareManager()->newShare();
+		$share = \OCP\Server::get(IManager::class)->newShare();
 		$share->setShareType(IShare::TYPE_EMAIL)
 			->setSharedBy('initiator')
 			->setSharedWith('user@server.com')
@@ -4765,7 +4768,7 @@ class ShareAPIControllerTest extends TestCase {
 		];
 
 		// Preview is available
-		$share = \OC::$server->getShareManager()->newShare();
+		$share = \OCP\Server::get(IManager::class)->newShare();
 		$share->setShareType(IShare::TYPE_USER)
 			->setSharedWith('recipient')
 			->setSharedBy('initiator')
@@ -4931,7 +4934,7 @@ class ShareAPIControllerTest extends TestCase {
 
 		$result = [];
 
-		$share = \OC::$server->getShareManager()->newShare();
+		$share = \OCP\Server::get(IManager::class)->newShare();
 		$share->setShareType(IShare::TYPE_ROOM)
 			->setSharedWith('recipientRoom')
 			->setSharedBy('initiator')
@@ -4983,7 +4986,7 @@ class ShareAPIControllerTest extends TestCase {
 			], $share, false, []
 		];
 
-		$share = \OC::$server->getShareManager()->newShare();
+		$share = \OCP\Server::get(IManager::class)->newShare();
 		$share->setShareType(IShare::TYPE_ROOM)
 			->setSharedWith('recipientRoom')
 			->setSharedBy('initiator')
@@ -5089,6 +5092,9 @@ class ShareAPIControllerTest extends TestCase {
 		$this->assertEquals($expects, $result);
 	}
 
+	/**
+	 * @return list{Folder, Folder}
+	 */
 	private function getNonSharedUserFolder(): array {
 		$node = $this->getMockBuilder(Folder::class)->getMock();
 		$userFolder = $this->getMockBuilder(Folder::class)->getMock();
@@ -5104,6 +5110,9 @@ class ShareAPIControllerTest extends TestCase {
 		return [$userFolder, $node];
 	}
 
+	/**
+	 * @return list{Folder, File}
+	 */
 	private function getNonSharedUserFile(): array {
 		$node = $this->getMockBuilder(File::class)->getMock();
 		$userFolder = $this->getMockBuilder(Folder::class)->getMock();
diff --git a/build/integration/features/bootstrap/SharingContext.php b/build/integration/features/bootstrap/SharingContext.php
index 97c2a35ad84..d2f1a2446ae 100644
--- a/build/integration/features/bootstrap/SharingContext.php
+++ b/build/integration/features/bootstrap/SharingContext.php
@@ -27,6 +27,7 @@ class SharingContext implements Context, SnippetAcceptingContext {
 		$this->deleteServerConfig('core', 'shareapi_default_expire_date');
 		$this->deleteServerConfig('core', 'shareapi_expire_after_n_days');
 		$this->deleteServerConfig('core', 'link_defaultExpDays');
+		$this->deleteServerConfig('files_sharing', 'outgoing_server2server_share_enabled');
 
 		$this->runOcc(['config:system:delete', 'share_folder']);
 	}
diff --git a/build/integration/sharing_features/sharing-v1-part4.feature b/build/integration/sharing_features/sharing-v1-part4.feature
index 1b32f99a0a2..d138f0a1769 100644
--- a/build/integration/sharing_features/sharing-v1-part4.feature
+++ b/build/integration/sharing_features/sharing-v1-part4.feature
@@ -124,3 +124,61 @@ Scenario: Receiving a share of a file without delete permission gives delete per
       | path | /welcome (2).txt |
       | permissions | 19 |
       | item_permissions | 27 |
+
+# This is a regression test as in the past creating a file drop required creating with permissions=5
+# and then afterwards update the share to permissions=4
+Scenario: Directly create link share with CREATE only permissions (file drop)
+    Given user "user0" exists
+    And As an "user0"
+    And user "user0" created a folder "/TMP"
+    When creating a share with
+      | path        | TMP |
+      | shareType   |   3 |
+      | permissions |   4 |
+    And Getting info of last share
+    Then Share fields of last share match with
+      | uid_file_owner | user0 |
+      | share_type     |     3 |
+      | permissions    |     4 |
+
+Scenario: Directly create email share with CREATE only permissions (file drop)
+  Given user "user0" exists
+  And As an "user0"
+  And user "user0" created a folder "/TMP"
+  When creating a share with
+    | path        |               TMP |
+    | shareType   |                 4 |
+    | shareWith   | j.doe@example.com |
+    | permissions |                 4 |
+  And Getting info of last share
+  Then Share fields of last share match with
+    | uid_file_owner | user0 |
+    | share_type     |     4 |
+    | permissions    |     4 |
+
+# This ensures the legacy behavior of sharing v1 is kept
+Scenario: publicUpload overrides permissions
+    Given user "user0" exists
+    And As an "user0"
+    And parameter "outgoing_server2server_share_enabled" of app "files_sharing" is set to "no"
+    And user "user0" created a folder "/TMP"
+    When creating a share with
+      | path         |  TMP |
+      | shareType    |    3 |
+      | permissions  |    4 |
+      | publicUpload | true |
+    And Getting info of last share
+    Then Share fields of last share match with
+      | uid_file_owner | user0 |
+      | share_type     |     3 |
+      | permissions    |    15 |
+    When creating a share with
+      | path         |   TMP |
+      | shareType    |     3 |
+      | permissions  |     4 |
+      | publicUpload | false |
+    And Getting info of last share
+    Then Share fields of last share match with
+      | uid_file_owner | user0 |
+      | share_type     |     3 |
+      | permissions    |     1 |
diff --git a/lib/private/Share20/Manager.php b/lib/private/Share20/Manager.php
index 35166c37ee3..a84481730c4 100644
--- a/lib/private/Share20/Manager.php
+++ b/lib/private/Share20/Manager.php
@@ -217,6 +217,17 @@ class Manager implements IManager {
 			throw new \InvalidArgumentException($this->l->t('Valid permissions are required for sharing'));
 		}
 
+		// Permissions must be valid
+		if ($share->getPermissions() < 0 || $share->getPermissions() > \OCP\Constants::PERMISSION_ALL) {
+			throw new \InvalidArgumentException($this->l->t('Valid permissions are required for sharing'));
+		}
+
+		// Single file shares should never have delete or create permissions
+		if (($share->getNode() instanceof File)
+			&& (($share->getPermissions() & (\OCP\Constants::PERMISSION_CREATE | \OCP\Constants::PERMISSION_DELETE)) !== 0)) {
+			throw new \InvalidArgumentException($this->l->t('File shares cannot have create or delete permissions'));
+		}
+
 		$permissions = 0;
 		$nodesForUser = $userFolder->getById($share->getNodeId());
 		foreach ($nodesForUser as $node) {
diff --git a/tests/lib/Share20/ManagerTest.php b/tests/lib/Share20/ManagerTest.php
index 6df97423989..8b8d6ea4e65 100644
--- a/tests/lib/Share20/ManagerTest.php
+++ b/tests/lib/Share20/ManagerTest.php
@@ -893,10 +893,9 @@ class ManagerTest extends \Test\TestCase {
 		$mount = $this->createMock(MoveableMount::class);
 		$limitedPermssions->method('getMountPoint')->willReturn($mount);
 
-
-		$data[] = [$this->createShare(null, IShare::TYPE_USER, $limitedPermssions, $user2, $user0, $user0, 31, null, null), 'Cannot increase permissions of path', true];
+		// increase permissions of a re-share
 		$data[] = [$this->createShare(null, IShare::TYPE_GROUP, $limitedPermssions, $group0, $user0, $user0, 17, null, null), 'Cannot increase permissions of path', true];
-		$data[] = [$this->createShare(null, IShare::TYPE_LINK, $limitedPermssions, null, $user0, $user0, 3, null, null), 'Cannot increase permissions of path', true];
+		$data[] = [$this->createShare(null, IShare::TYPE_USER, $limitedPermssions, $user2, $user0, $user0, 3, null, null), 'Cannot increase permissions of path', true];
 
 		$nonMovableStorage = $this->createMock(IStorage::class);
 		$nonMovableStorage->method('instanceOfStorage')
@@ -927,6 +926,20 @@ class ManagerTest extends \Test\TestCase {
 		$data[] = [$this->createShare(null, IShare::TYPE_GROUP, $rootFolder, $group0, $user0, $user0, 2, null, null), 'You cannot share your root folder', true];
 		$data[] = [$this->createShare(null, IShare::TYPE_LINK, $rootFolder, null, $user0, $user0, 16, null, null), 'You cannot share your root folder', true];
 
+		$allPermssionsFiles = $this->createMock(File::class);
+		$allPermssionsFiles->method('isShareable')->willReturn(true);
+		$allPermssionsFiles->method('getPermissions')->willReturn(\OCP\Constants::PERMISSION_ALL);
+		$allPermssionsFiles->method('getId')->willReturn(187);
+		$allPermssionsFiles->method('getOwner')
+			->willReturn($owner);
+		$allPermssionsFiles->method('getStorage')
+			->willReturn($storage);
+
+		// test invalid CREATE or DELETE permissions
+		$data[] = [$this->createShare(null, IShare::TYPE_USER, $allPermssionsFiles, $user2, $user0, $user0, \OCP\Constants::PERMISSION_ALL, null, null), 'File shares cannot have create or delete permissions', true];
+		$data[] = [$this->createShare(null, IShare::TYPE_GROUP, $allPermssionsFiles, $group0, $user0, $user0, \OCP\Constants::PERMISSION_READ | \OCP\Constants::PERMISSION_CREATE, null, null), 'File shares cannot have create or delete permissions', true];
+		$data[] = [$this->createShare(null, IShare::TYPE_LINK, $allPermssionsFiles, null, $user0, $user0, \OCP\Constants::PERMISSION_READ | \OCP\Constants::PERMISSION_DELETE, null, null), 'File shares cannot have create or delete permissions', true];
+
 		$allPermssions = $this->createMock(Folder::class);
 		$allPermssions->method('isShareable')->willReturn(true);
 		$allPermssions->method('getPermissions')->willReturn(\OCP\Constants::PERMISSION_ALL);
@@ -939,6 +952,12 @@ class ManagerTest extends \Test\TestCase {
 		$data[] = [$this->createShare(null, IShare::TYPE_USER, $allPermssions, $user2, $user0, $user0, 30, null, null), 'Shares need at least read permissions', true];
 		$data[] = [$this->createShare(null, IShare::TYPE_GROUP, $allPermssions, $group0, $user0, $user0, 2, null, null), 'Shares need at least read permissions', true];
 
+		// test invalid permissions
+		$data[] = [$this->createShare(null, IShare::TYPE_USER, $allPermssions, $user2, $user0, $user0, 32, null, null), 'Valid permissions are required for sharing', true];
+		$data[] = [$this->createShare(null, IShare::TYPE_GROUP, $allPermssions, $group0, $user0, $user0, 63, null, null), 'Valid permissions are required for sharing', true];
+		$data[] = [$this->createShare(null, IShare::TYPE_LINK, $allPermssions, null, $user0, $user0, -1, null, null), 'Valid permissions are required for sharing', true];
+
+		// working shares
 		$data[] = [$this->createShare(null, IShare::TYPE_USER, $allPermssions, $user2, $user0, $user0, 31, null, null), null, false];
 		$data[] = [$this->createShare(null, IShare::TYPE_GROUP, $allPermssions, $group0, $user0, $user0, 3, null, null), null, false];
 		$data[] = [$this->createShare(null, IShare::TYPE_LINK, $allPermssions, null, $user0, $user0, 17, null, null), null, false];
@@ -2406,8 +2425,9 @@ class ManagerTest extends \Test\TestCase {
 	}
 
 	public function testCreateShareUser(): void {
+		/** @var Manager&MockObject $manager */
 		$manager = $this->createManagerMock()
-			->setMethods(['canShare', 'generalCreateChecks', 'userCreateChecks', 'pathCreateChecks'])
+			->onlyMethods(['canShare', 'generalCreateChecks', 'userCreateChecks', 'pathCreateChecks'])
 			->getMock();
 
 		$shareOwner = $this->createMock(IUser::class);