Merge pull request #51602 from nextcloud/fix/fix-default-share-folder-for-group-shares

Author: provokateurin

lib/private/Share20/DefaultShareProvider.php

@@ -13,12 +13,14 @@
 use OC\Share20\Exception\InvalidShare;
 use OC\Share20\Exception\ProviderException;
 use OC\User\LazyUser;
+use OCA\Files_Sharing\AppInfo\Application;
 use OCP\AppFramework\Utility\ITimeFactory;
 use OCP\DB\QueryBuilder\IQueryBuilder;
 use OCP\Defaults;
 use OCP\Files\Folder;
 use OCP\Files\IRootFolder;
 use OCP\Files\Node;
+use OCP\IConfig;
 use OCP\IDBConnection;
 use OCP\IGroupManager;
 use OCP\IL10N;
@@ -58,6 +60,7 @@ public function __construct(
 		private ITimeFactory $timeFactory,
 		private LoggerInterface $logger,
 		private IManager $shareManager,
+		private IConfig $config,
 	) {
 	}
 
@@ -485,6 +488,15 @@ public function deleteFromSelf(IShare $share, $recipient) {
 	protected function createUserSpecificGroupShare(IShare $share, string $recipient): int {
 		$type = $share->getNodeType();
 
+		$shareFolder = $this->config->getSystemValue('share_folder', '/');
+		$allowCustomShareFolder = $this->config->getSystemValueBool('sharing.allow_custom_share_folder', true);
+		if ($allowCustomShareFolder) {
+			$shareFolder = $this->config->getUserValue($recipient, Application::APP_ID, 'share_folder', $shareFolder);
+		}
+
+		$target = $shareFolder . '/' . $share->getNode()->getName();
+		$target = \OC\Files\Filesystem::normalizePath($target);
+
 		$qb = $this->dbConn->getQueryBuilder();
 		$qb->insert('share')
 			->values([
@@ -496,7 +508,7 @@ protected function createUserSpecificGroupShare(IShare $share, string $recipient
 				'item_type' => $qb->createNamedParameter($type),
 				'item_source' => $qb->createNamedParameter($share->getNodeId()),
 				'file_source' => $qb->createNamedParameter($share->getNodeId()),
-				'file_target' => $qb->createNamedParameter($share->getTarget()),
+				'file_target' => $qb->createNamedParameter($target),
 				'permissions' => $qb->createNamedParameter($share->getPermissions()),
 				'stime' => $qb->createNamedParameter($share->getShareTime()->getTimestamp()),
 			])->executeStatement();

lib/private/Share20/Manager.php

@@ -705,12 +705,12 @@ public function createShare(IShare $share) {
 			}
 
 			// Generate the target
-			$defaultShareFolder = $this->config->getSystemValue('share_folder', '/');
-			$allowCustomShareFolder = $this->config->getSystemValueBool('sharing.allow_custom_share_folder', true);
-			if ($allowCustomShareFolder) {
-				$shareFolder = $this->config->getUserValue($share->getSharedWith(), Application::APP_ID, 'share_folder', $defaultShareFolder);
-			} else {
-				$shareFolder = $defaultShareFolder;
+			$shareFolder = $this->config->getSystemValue('share_folder', '/');
+			if ($share->getShareType() === IShare::TYPE_USER) {
+				$allowCustomShareFolder = $this->config->getSystemValueBool('sharing.allow_custom_share_folder', true);
+				if ($allowCustomShareFolder) {
+					$shareFolder = $this->config->getUserValue($share->getSharedWith(), Application::APP_ID, 'share_folder', $shareFolder);
+				}
 			}
 
 			$target = $shareFolder . '/' . $share->getNode()->getName();

tests/lib/Share20/DefaultShareProviderTest.php

@@ -20,6 +20,7 @@
 use OCP\Files\File;
 use OCP\Files\Folder;
 use OCP\Files\IRootFolder;
+use OCP\IConfig;
 use OCP\IDBConnection;
 use OCP\IGroup;
 use OCP\IGroupManager;
@@ -79,6 +80,8 @@ class DefaultShareProviderTest extends \Test\TestCase {
 	/** @var LoggerInterface|MockObject */
 	protected $logger;
 
+	protected IConfig&MockObject $config;
+
 	protected IShareManager&MockObject $shareManager;
 
 	protected function setUp(): void {
@@ -94,6 +97,7 @@ protected function setUp(): void {
 		$this->timeFactory = $this->createMock(ITimeFactory::class);
 		$this->logger = $this->createMock(LoggerInterface::class);
 		$this->shareManager = $this->createMock(IShareManager::class);
+		$this->config = $this->createMock(IConfig::class);
 
 		$this->userManager->expects($this->any())->method('userExists')->willReturn(true);
 		$this->timeFactory->expects($this->any())->method('now')->willReturn(new \DateTimeImmutable('2023-05-04 00:00 Europe/Berlin'));
@@ -113,6 +117,7 @@ protected function setUp(): void {
 			$this->timeFactory,
 			$this->logger,
 			$this->shareManager,
+			$this->config,
 		);
 	}
 
@@ -477,6 +482,7 @@ public function testDeleteSingleShare(): void {
 				$this->timeFactory,
 				$this->logger,
 				$this->shareManager,
+				$this->config,
 			])
 			->onlyMethods(['getShareById'])
 			->getMock();
@@ -574,6 +580,7 @@ public function testDeleteGroupShareWithUserGroupShares(): void {
 				$this->timeFactory,
 				$this->logger,
 				$this->shareManager,
+				$this->config,
 			])
 			->onlyMethods(['getShareById'])
 			->getMock();
@@ -2566,6 +2573,7 @@ public function testGetSharesInFolder(): void {
 			$this->timeFactory,
 			$this->logger,
 			$this->shareManager,
+			$this->config,
 		);
 
 		$password = md5(time());
@@ -2666,6 +2674,7 @@ public function testGetAccessListNoCurrentAccessRequired(): void {
 			$this->timeFactory,
 			$this->logger,
 			$this->shareManager,
+			$this->config,
 		);
 
 		$u1 = $userManager->createUser('testShare1', 'test');
@@ -2769,6 +2778,7 @@ public function testGetAccessListCurrentAccessRequired(): void {
 			$this->timeFactory,
 			$this->logger,
 			$this->shareManager,
+			$this->config,
 		);
 
 		$u1 = $userManager->createUser('testShare1', 'test');