[stable9] Group shares with same source and target

apps/files_sharing/lib/mountprovider.php

@@ -50,6 +50,15 @@ public function __construct(IConfig $config, ILogger $logger) {
 		$this->logger = $logger;
 	}
 
+	/**
+	 * Return items shared with user
+	 *
+	 * @internal
+	 */
+	public function getItemsSharedWithUser($uid) {
+		// only here to make it mockable/testable
+		return \OCP\Share::getItemsSharedWithUser('file', $uid, \OCP\Share::FORMAT_NONE, null, -1, false, true);
+	}
 
 	/**
 	 * Get all mountpoints applicable for the user and check for shares where we need to update the etags
@@ -59,7 +68,7 @@ public function __construct(IConfig $config, ILogger $logger) {
 	 * @return \OCP\Files\Mount\IMountPoint[]
 	 */
 	public function getMountsForUser(IUser $user, IStorageFactory $storageFactory) {
-		$shares = \OCP\Share::getItemsSharedWithUser('file', $user->getUID());
+		$shares = $this->getItemsSharedWithUser($user->getUID());
 		$shares = array_filter($shares, function ($share) {
 			return $share['permissions'] > 0;
 		});

apps/files_sharing/tests/mountprovider.php

@@ -0,0 +1,146 @@
+<?php
+/**
+ * @author Vincent Petry <pvince81@owncloud.com>
+ *
+ * @copyright Copyright (c) 2016, ownCloud, Inc.
+ * @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\Files_Sharing\Tests;
+
+use OCA\Files_Sharing\MountProvider;
+use OCP\Files\Storage\IStorageFactory;
+use OCP\IConfig;
+use OCP\ILogger;
+use OCP\IUser;
+use OCP\Share\IShare;
+use OCP\Share\IManager;
+use OCP\Files\Mount\IMountPoint;
+
+/**
+ * @group DB
+ */
+class MountProviderTest extends \Test\TestCase {
+
+	/** @var MountProvider */
+	private $provider;
+
+	/** @var IConfig|\PHPUnit_Framework_MockObject_MockObject */
+	private $config;
+
+	/** @var IUser|\PHPUnit_Framework_MockObject_MockObject */
+	private $user;
+
+	/** @var IStorageFactory|\PHPUnit_Framework_MockObject_MockObject */
+	private $loader;
+
+	/** @var ILogger | \PHPUnit_Framework_MockObject_MockObject */
+	private $logger;
+
+	public function setUp() {
+		parent::setUp();
+
+		$this->config = $this->getMock('OCP\IConfig');
+		$this->user = $this->getMock('OCP\IUser');
+		$this->loader = $this->getMock('OCP\Files\Storage\IStorageFactory');
+		$this->loader->expects($this->any())
+			->method('getInstance')
+			->will($this->returnCallback(function($mountPoint, $class, $arguments) {
+				$storage = $this->getMockBuilder('OC\Files\Storage\Shared')
+					->disableOriginalConstructor()
+					->getMock();
+				$storage->expects($this->any())
+					->method('getShare')
+					->will($this->returnValue($arguments['share']));
+				return $storage;
+			}));
+		$this->logger = $this->getMock('\OCP\ILogger');
+		$this->logger->expects($this->never())
+			->method('error');
+
+		$this->provider = $this->getMockBuilder('OCA\Files_Sharing\MountProvider')
+			->setMethods(['getItemsSharedWithUser'])
+			->setConstructorArgs([$this->config, $this->logger])
+			->getMock();
+	}
+
+	private function makeMockShare($id, $nodeId, $owner = 'user2', $target = null, $permissions = 31, $shareType) {
+		return [
+			'id' => $id,
+			'uid_owner' => $owner,
+			'share_type' => $shareType,
+			'item_type' => 'file',
+			'file_target' => $target,
+			'file_source' => $nodeId,
+			'item_target' => null,
+			'item_source' => $nodeId,
+			'permissions' => $permissions,
+			'stime' => time(),
+			'token' => null,
+			'expiration' => null,
+		];
+	}
+
+	/**
+	 * Tests excluding shares from the current view. This includes:
+	 * - shares that were opted out of (permissions === 0)
+	 * - shares with a group in which the owner is already in
+	 */
+	public function testExcludeShares() {
+		$userShares = [
+			$this->makeMockShare(1, 100, 'user2', '/share2', 0, \OCP\Share::SHARE_TYPE_USER), 
+			$this->makeMockShare(2, 100, 'user2', '/share2', 31, \OCP\Share::SHARE_TYPE_USER),
+		];
+
+		$groupShares = [
+			$this->makeMockShare(3, 100, 'user2', '/share2', 0, \OCP\Share::SHARE_TYPE_GROUP), 
+			$this->makeMockShare(4, 100, 'user2', '/share4', 31, \OCP\Share::SHARE_TYPE_GROUP), 
+		];
+
+		$this->user->expects($this->any())
+			->method('getUID')
+			->will($this->returnValue('user1'));
+
+		$allShares = array_merge($userShares, $groupShares);
+
+		$this->provider->expects($this->once())
+			->method('getItemsSharedWithUser')
+			->with('user1')
+			->will($this->returnValue($allShares));
+
+		$mounts = $this->provider->getMountsForUser($this->user, $this->loader);
+
+		$this->assertCount(2, $mounts);
+		$this->assertInstanceOf('OCA\Files_Sharing\SharedMount', $mounts[0]);
+		$this->assertInstanceOf('OCA\Files_Sharing\SharedMount', $mounts[1]);
+
+		$mountedShare1 = $mounts[0]->getShare();
+
+		$this->assertEquals('2', $mountedShare1['id']);
+		$this->assertEquals('user2', $mountedShare1['uid_owner']);
+		$this->assertEquals(100, $mountedShare1['file_source']);
+		$this->assertEquals('/share2', $mountedShare1['file_target']);
+		$this->assertEquals(31, $mountedShare1['permissions']);
+
+		$mountedShare2 = $mounts[1]->getShare();
+		$this->assertEquals('4', $mountedShare2['id']);
+		$this->assertEquals('user2', $mountedShare2['uid_owner']);
+		$this->assertEquals(100, $mountedShare2['file_source']);
+		$this->assertEquals('/share4', $mountedShare2['file_target']);
+		$this->assertEquals(31, $mountedShare2['permissions']);
+	}
+}
+

build/integration/features/sharing-v1.feature

@@ -608,3 +608,146 @@ Feature: sharing
       | /foo/       |
       | /foo%20(2)/ |
 
+  Scenario: Merging shares for recipient when shared from outside with group and member
+    Given As an "admin"
+    And user "user0" exists
+    And user "user1" exists
+    And group "group1" exists
+    And user "user1" belongs to group "group1"
+    And user "user0" created a folder "merge-test-outside"
+    When folder "merge-test-outside" of user "user0" is shared with group "group1"
+    And folder "merge-test-outside" of user "user0" is shared with user "user1"
+    Then as "user1" the folder "merge-test-outside" exists
+    And as "user1" the folder "merge-test-outside (2)" does not exist
+
+  Scenario: Merging shares for recipient when shared from outside with group and member with different permissions
+    Given As an "admin"
+    And user "user0" exists
+    And user "user1" exists
+    And group "group1" exists
+    And user "user1" belongs to group "group1"
+    And user "user0" created a folder "merge-test-outside-perms"
+    When folder "merge-test-outside-perms" of user "user0" is shared with group "group1" with permissions 1
+    And folder "merge-test-outside-perms" of user "user0" is shared with user "user1" with permissions 31
+    Then as "user1" gets properties of folder "merge-test-outside-perms" with
+        |{http://owncloud.org/ns}permissions|
+    And the single response should contain a property "{http://owncloud.org/ns}permissions" with value "SRDNVCK"
+    And as "user1" the folder "merge-test-outside-perms (2)" does not exist
+
+  Scenario: Merging shares for recipient when shared from outside with two groups
+    Given As an "admin"
+    And user "user0" exists
+    And user "user1" exists
+    And group "group1" exists
+    And group "group2" exists
+    And user "user1" belongs to group "group1"
+    And user "user1" belongs to group "group2"
+    And user "user0" created a folder "merge-test-outside-twogroups"
+    When folder "merge-test-outside-twogroups" of user "user0" is shared with group "group1"
+    And folder "merge-test-outside-twogroups" of user "user0" is shared with group "group2"
+    Then as "user1" the folder "merge-test-outside-twogroups" exists
+    And as "user1" the folder "merge-test-outside-twogroups (2)" does not exist
+
+  Scenario: Merging shares for recipient when shared from outside with two groups with different permissions
+    Given As an "admin"
+    And user "user0" exists
+    And user "user1" exists
+    And group "group1" exists
+    And group "group2" exists
+    And user "user1" belongs to group "group1"
+    And user "user1" belongs to group "group2"
+    And user "user0" created a folder "merge-test-outside-twogroups-perms"
+    When folder "merge-test-outside-twogroups-perms" of user "user0" is shared with group "group1" with permissions 1
+    And folder "merge-test-outside-twogroups-perms" of user "user0" is shared with group "group2" with permissions 31
+    Then as "user1" gets properties of folder "merge-test-outside-twogroups-perms" with
+        |{http://owncloud.org/ns}permissions|
+    And the single response should contain a property "{http://owncloud.org/ns}permissions" with value "SRDNVCK"
+    And as "user1" the folder "merge-test-outside-twogroups-perms (2)" does not exist
+
+  Scenario: Merging shares for recipient when shared from outside with two groups and member
+    Given As an "admin"
+    And user "user0" exists
+    And user "user1" exists
+    And group "group1" exists
+    And group "group2" exists
+    And user "user1" belongs to group "group1"
+    And user "user1" belongs to group "group2"
+    And user "user0" created a folder "merge-test-outside-twogroups-member-perms"
+    When folder "merge-test-outside-twogroups-member-perms" of user "user0" is shared with group "group1" with permissions 1
+    And folder "merge-test-outside-twogroups-member-perms" of user "user0" is shared with group "group2" with permissions 31
+    And folder "merge-test-outside-twogroups-member-perms" of user "user0" is shared with user "user1" with permissions 1
+    Then as "user1" gets properties of folder "merge-test-outside-twogroups-member-perms" with
+        |{http://owncloud.org/ns}permissions|
+    And the single response should contain a property "{http://owncloud.org/ns}permissions" with value "SRDNVCK"
+    And as "user1" the folder "merge-test-outside-twogroups-member-perms (2)" does not exist
+
+  Scenario: Merging shares for recipient when shared from inside with group
+    Given As an "admin"
+    And user "user0" exists
+    And group "group1" exists
+    And user "user0" belongs to group "group1"
+    And user "user0" created a folder "merge-test-inside-group"
+    When folder "/merge-test-inside-group" of user "user0" is shared with group "group1"
+    Then as "user0" the folder "merge-test-inside-group" exists
+    And as "user0" the folder "merge-test-inside-group (2)" does not exist
+
+  Scenario: Merging shares for recipient when shared from inside with two groups
+    Given As an "admin"
+    And user "user0" exists
+    And group "group1" exists
+    And group "group2" exists
+    And user "user0" belongs to group "group1"
+    And user "user0" belongs to group "group2"
+    And user "user0" created a folder "merge-test-inside-twogroups"
+    When folder "merge-test-inside-twogroups" of user "user0" is shared with group "group1"
+    And folder "merge-test-inside-twogroups" of user "user0" is shared with group "group2"
+    Then as "user0" the folder "merge-test-inside-twogroups" exists
+    And as "user0" the folder "merge-test-inside-twogroups (2)" does not exist
+    And as "user0" the folder "merge-test-inside-twogroups (3)" does not exist
+
+  Scenario: Merging shares for recipient when shared from inside with group with less permissions
+    Given As an "admin"
+    And user "user0" exists
+    And group "group1" exists
+    And group "group2" exists
+    And user "user0" belongs to group "group1"
+    And user "user0" belongs to group "group2"
+    And user "user0" created a folder "merge-test-inside-twogroups-perms"
+    When folder "merge-test-inside-twogroups-perms" of user "user0" is shared with group "group1"
+    And folder "merge-test-inside-twogroups-perms" of user "user0" is shared with group "group2"
+    Then as "user0" gets properties of folder "merge-test-inside-twogroups-perms" with
+        |{http://owncloud.org/ns}permissions|
+    And the single response should contain a property "{http://owncloud.org/ns}permissions" with value "RDNVCK"
+    And as "user0" the folder "merge-test-inside-twogroups-perms (2)" does not exist
+    And as "user0" the folder "merge-test-inside-twogroups-perms (3)" does not exist
+
+  Scenario: Merging shares for recipient when shared from outside with group then user and recipient renames in between
+    Given As an "admin"
+    And user "user0" exists
+    And user "user1" exists
+    And group "group1" exists
+    And user "user1" belongs to group "group1"
+    And user "user0" created a folder "merge-test-outside-groups-renamebeforesecondshare"
+    When folder "merge-test-outside-groups-renamebeforesecondshare" of user "user0" is shared with group "group1"
+    And User "user1" moved folder "/merge-test-outside-groups-renamebeforesecondshare" to "/merge-test-outside-groups-renamebeforesecondshare-renamed"
+    And folder "merge-test-outside-groups-renamebeforesecondshare" of user "user0" is shared with user "user1"
+    Then as "user1" gets properties of folder "merge-test-outside-groups-renamebeforesecondshare-renamed" with
+        |{http://owncloud.org/ns}permissions|
+    And the single response should contain a property "{http://owncloud.org/ns}permissions" with value "SRDNVCK"
+    And as "user1" the folder "merge-test-outside-groups-renamebeforesecondshare" does not exist
+
+  Scenario: Merging shares for recipient when shared from outside with user then group and recipient renames in between
+    Given As an "admin"
+    And user "user0" exists
+    And user "user1" exists
+    And group "group1" exists
+    And user "user1" belongs to group "group1"
+    And user "user0" created a folder "merge-test-outside-groups-renamebeforesecondshare"
+    When folder "merge-test-outside-groups-renamebeforesecondshare" of user "user0" is shared with user "user1"
+    And User "user1" moved folder "/merge-test-outside-groups-renamebeforesecondshare" to "/merge-test-outside-groups-renamebeforesecondshare-renamed"
+    And folder "merge-test-outside-groups-renamebeforesecondshare" of user "user0" is shared with group "group1"
+    Then as "user1" gets properties of folder "merge-test-outside-groups-renamebeforesecondshare-renamed" with
+        |{http://owncloud.org/ns}permissions|
+    And the single response should contain a property "{http://owncloud.org/ns}permissions" with value "SRDNVCK"
+    And as "user1" the folder "merge-test-outside-groups-renamebeforesecondshare" does not exist
+

core/js/shareitemmodel.js

@@ -601,6 +601,33 @@
 			}
 		},
 
+		/**
+		 * Group reshares into a single super share element.
+		 * Does this by finding the most precise share and
+		 * combines the permissions to be the most permissive.
+		 *
+		 * @param {Array} reshares
+		 * @return {Object} reshare
+		 */
+		_groupReshares: function(reshares) {
+			if (!reshares || !reshares.length) {
+				return false;
+			}
+
+			var superShare = reshares.shift();
+			var combinedPermissions = superShare.permissions;
+			_.each(reshares, function(reshare) {
+				// use share have higher priority than group share
+				if (reshare.share_type === OC.Share.SHARE_TYPE_USER && superShare.share_type === OC.Share.SHARE_TYPE_GROUP) {
+					superShare = reshare;
+				}
+				combinedPermissions |= reshare.permissions;
+			});
+
+			superShare.permissions = combinedPermissions;
+			return superShare;
+		},
+
 		fetch: function() {
 			var model = this;
 			this.trigger('request', this);
@@ -618,7 +645,7 @@
 
 				var reshare = false;
 				if (data2[0].ocs.data.length) {
-					reshare = data2[0].ocs.data[0];
+					reshare = model._groupReshares(data2[0].ocs.data);
 				}
 
 				model.set(model.parse({