Merge pull request #7328 from nextcloud/backport/7327/access-list-regression-for-not-current-accesss

[stable12]  Only in case of $currentAccess the array uses the id as index

2 files changed, 120 insertions, 3 deletions

diff --git a/lib/private/Share20/Manager.php b/lib/private/Share20/Manager.php
index be019852ff3..46b47920ce1 100644
--- a/lib/private/Share20/Manager.php
+++ b/lib/private/Share20/Manager.php
@@ -1407,7 +1407,13 @@ class Manager implements IManager {
 			foreach ($tmp as $k => $v) {
 				if (isset($al[$k])) {
 					if (is_array($al[$k])) {
-						$al[$k] += $v;
+						if ($currentAccess) {
+							$al[$k] += $v;
+						} else {
+							$al[$k] = array_merge($al[$k], $v);
+							$al[$k] = array_unique($al[$k]);
+							$al[$k] = array_values($al[$k]);
+						}
 					} else {
 						$al[$k] = $al[$k] || $v;
 					}
diff --git a/tests/lib/Share20/ManagerTest.php b/tests/lib/Share20/ManagerTest.php
index 95831d19cd1..c2dda4dcf85 100644
--- a/tests/lib/Share20/ManagerTest.php
+++ b/tests/lib/Share20/ManagerTest.php
@@ -2938,6 +2938,115 @@ class ManagerTest extends \Test\TestCase {
 		$this->defaultProvider->method('getAccessList')
 			->with(
 				$this->equalTo([$file, $folder]),
+				false
+			)
+			->willReturn([
+				'users' => [
+					'user1',
+					'user2',
+					'user3',
+					'123456',
+				],
+				'public' => true,
+			]);
+
+		$extraProvider->method('getAccessList')
+			->with(
+				$this->equalTo([$file, $folder]),
+				false
+			)
+			->willReturn([
+				'users' => [
+					'user3',
+					'user4',
+					'user5',
+					'234567',
+				],
+				'remote' => true,
+			]);
+
+		$this->rootFolder->method('getUserFolder')
+			->with($this->equalTo('owner'))
+			->willReturn($userFolder);
+
+		$expected = [
+			'users' => ['owner', 'user1', 'user2', 'user3', '123456','user4', 'user5', '234567'],
+			'remote' => true,
+			'public' => true,
+		];
+
+		$result = $manager->getAccessList($node, true, false);
+
+		$this->assertSame($expected['public'], $result['public']);
+		$this->assertSame($expected['remote'], $result['remote']);
+		$this->assertSame($expected['users'], $result['users']);
+
+	}
+
+	public function testGetAccessListWithCurrentAccess() {
+		$factory = new DummyFactory2($this->createMock(IServerContainer::class));
+
+		$manager = new Manager(
+			$this->logger,
+			$this->config,
+			$this->secureRandom,
+			$this->hasher,
+			$this->mountManager,
+			$this->groupManager,
+			$this->l,
+			$this->l10nFactory,
+			$factory,
+			$this->userManager,
+			$this->rootFolder,
+			$this->eventDispatcher,
+			$this->mailer,
+			$this->urlGenerator,
+			$this->defaults
+		);
+
+		$factory->setProvider($this->defaultProvider);
+		$extraProvider = $this->createMock(IShareProvider::class);
+		$factory->setSecondProvider($extraProvider);
+
+		$owner = $this->createMock(IUser::class);
+		$owner->expects($this->once())
+			->method('getUID')
+			->willReturn('owner');
+
+		$node = $this->createMock(Node::class);
+		$node->expects($this->once())
+			->method('getOwner')
+			->willReturn($owner);
+		$node->method('getId')
+			->willReturn(42);
+
+		$userFolder = $this->createMock(Folder::class);
+		$file = $this->createMock(File::class);
+		$folder = $this->createMock(Folder::class);
+
+		$file->method('getParent')
+			->willReturn($folder);
+		$file->method('getPath')
+			->willReturn('/owner/files/folder/file');
+		$file->method('getId')
+			->willReturn(23);
+		$folder->method('getParent')
+			->willReturn($userFolder);
+		$folder->method('getPath')
+			->willReturn('/owner/files/folder');
+		$userFolder->method('getById')
+			->with($this->equalTo(42))
+			->willReturn([$file]);
+		$userFolder->method('getPath')
+			->willReturn('/owner/files');
+
+		$this->userManager->method('userExists')
+			->with($this->equalTo('owner'))
+			->willReturn(true);
+
+		$this->defaultProvider->method('getAccessList')
+			->with(
+				$this->equalTo([$file, $folder]),
 				true
 			)
 			->willReturn([
@@ -2945,6 +3054,7 @@ class ManagerTest extends \Test\TestCase {
 					'user1' => [],
 					'user2' => [],
 					'user3' => [],
+					'123456' => [],
 				],
 				'public' => true,
 			]);
@@ -2959,6 +3069,7 @@ class ManagerTest extends \Test\TestCase {
 					'user3' => [],
 					'user4' => [],
 					'user5' => [],
+					'234567' => [],
 				],
 				'remote' => [
 					'remote1',
@@ -2975,7 +3086,7 @@ class ManagerTest extends \Test\TestCase {
 					'node_id' => 23,
 					'node_path' => '/folder/file'
 				]
-				, 'user1' => [], 'user2' => [], 'user3' => [], 'user4' => [], 'user5' => []],
+				, 'user1' => [], 'user2' => [], 'user3' => [], '123456' => [], 'user4' => [], 'user5' => [], '234567' => []],
 			'remote' => [
 				'remote1',
 			],
@@ -2986,7 +3097,7 @@ class ManagerTest extends \Test\TestCase {
 
 		$this->assertSame($expected['public'], $result['public']);
 		$this->assertSame($expected['remote'], $result['remote']);
-		$this->assertSame(array_values($expected['users']), array_values($result['users']));
+		$this->assertSame($expected['users'], $result['users']);
 
 	}
 }