Fix group sharing and sharing permissions - fixes #22932

apps/dav/lib/carddav/addressbook.php

@@ -27,10 +27,6 @@ use Sabre\DAV\PropPatch;
 
 class AddressBook extends \Sabre\CardDAV\AddressBook implements IShareable {
 
-	public function __construct(CardDavBackend $carddavBackend, array $addressBookInfo) {
-		parent::__construct($carddavBackend, $addressBookInfo);
-	}
-
 	/**
 	 * Updates the list of shares.
 	 *

apps/dav/lib/migration/migrateaddressbooks.php

@@ -115,11 +115,12 @@ class MigrateAddressbooks {
 
 		$add = array_map(function($s) {
 			$prefix = 'principal:principals/users/';
-			if ($s['share_type'] === 1) {
+			if ((int)$s['share_type'] === 1) {
 				$prefix = 'principal:principals/groups/';
 			}
 			return [
-				'href' => $prefix . $s['share_with']
+				'href' => $prefix . $s['share_with'],
+				'readOnly' => !((int)$s['permissions'] === 31)
 			];
 		}, $shares);
 

apps/dav/lib/migration/migratecalendars.php

@@ -116,11 +116,12 @@ class MigrateCalendars {
 
 		$add = array_map(function($s) {
 			$prefix = 'principal:principals/users/';
-			if ($s['share_type'] === 1) {
+			if ((int)$s['share_type'] === 1) {
 				$prefix = 'principal:principals/groups/';
 			}
 			return [
-				'href' => $prefix . $s['share_with']
+				'href' => $prefix . $s['share_with'],
+				'readOnly' => !((int)$s['permissions'] === 31)
 			];
 		}, $shares);
 

apps/dav/tests/unit/migration/migrateaddressbooktest.php

@@ -29,13 +29,21 @@ class MigrateAddressbookTest extends TestCase {
 
 	public function testMigration() {
 		/** @var AddressBookAdapter | \PHPUnit_Framework_MockObject_MockObject $adapter */
-		$adapter = $this->mockAdapter();
+		$adapter = $this->mockAdapter([
+			['share_type' => '1', 'share_with' => 'users', 'permissions' => '31'],
+			['share_type' => '2', 'share_with' => 'adam', 'permissions' => '1'],
+		]);
 
 		/** @var CardDavBackend | \PHPUnit_Framework_MockObject_MockObject $cardDav */
 		$cardDav = $this->getMockBuilder('\OCA\Dav\CardDAV\CardDAVBackend')->disableOriginalConstructor()->getMock();
-		$cardDav->method('createAddressBook')->willReturn(666);
+		$cardDav->expects($this->any())->method('createAddressBook')->willReturn(666);
+		$cardDav->expects($this->any())->method('getAddressBookById')->willReturn([]);
 		$cardDav->expects($this->once())->method('createAddressBook')->with('principals/users/test01', 'test_contacts');
 		$cardDav->expects($this->once())->method('createCard')->with(666, '63f0dd6c-39d5-44be-9d34-34e7a7441fc2.vcf', 'BEGIN:VCARD');
+		$cardDav->expects($this->once())->method('updateShares')->with($this->anything(), [
+			['href' => 'principal:principals/groups/users', 'readOnly' => false],
+			['href' => 'principal:principals/users/adam', 'readOnly' => true]
+		]);
 		/** @var ILogger $logger */
 		$logger = $this->getMockBuilder('\OCP\ILogger')->disableOriginalConstructor()->getMock();
 
@@ -48,7 +56,7 @@ class MigrateAddressbookTest extends TestCase {
 	 */
 	private function mockAdapter($shares = []) {
 		$adapter = $this->getMockBuilder('\OCA\Dav\Migration\AddressBookAdapter')->disableOriginalConstructor()->getMock();
-		$adapter->method('foreachBook')->willReturnCallback(function ($user, \Closure $callBack) {
+		$adapter->expects($this->any())->method('foreachBook')->willReturnCallback(function ($user, \Closure $callBack) {
 			$callBack([
 				'id' => 0,
 				'userid' => $user,
@@ -59,14 +67,14 @@ class MigrateAddressbookTest extends TestCase {
 				'active' => 1
 			]);
 		});
-		$adapter->method('foreachCard')->willReturnCallback(function ($addressBookId, \Closure $callBack) {
+		$adapter->expects($this->any())->method('foreachCard')->willReturnCallback(function ($addressBookId, \Closure $callBack) {
 			$callBack([
 				'userid' => $addressBookId,
 				'uri' => '63f0dd6c-39d5-44be-9d34-34e7a7441fc2.vcf',
 				'carddata' => 'BEGIN:VCARD'
 			]);
 		});
-		$adapter->method('getShares')->willReturn($shares);
+		$adapter->expects($this->any())->method('getShares')->willReturn($shares);
 		return $adapter;
 	}
 

apps/dav/tests/unit/migration/migratecalendartest.php

@@ -29,13 +29,20 @@ class MigrateCalendarTest extends TestCase {
 
 	public function testMigration() {
 		/** @var CalendarAdapter | \PHPUnit_Framework_MockObject_MockObject $adapter */
-		$adapter = $this->mockAdapter();
+		$adapter = $this->mockAdapter([
+			['share_type' => '1', 'share_with' => 'users', 'permissions' => '31'],
+			['share_type' => '2', 'share_with' => 'adam', 'permissions' => '1'],
+		]);
 
 		/** @var CalDavBackend | \PHPUnit_Framework_MockObject_MockObject $cardDav */
 		$cardDav = $this->getMockBuilder('\OCA\Dav\CalDAV\CalDAVBackend')->disableOriginalConstructor()->getMock();
-		$cardDav->method('createCalendar')->willReturn(666);
+		$cardDav->expects($this->any())->method('createCalendar')->willReturn(666);
 		$cardDav->expects($this->once())->method('createCalendar')->with('principals/users/test01', 'test_contacts');
 		$cardDav->expects($this->once())->method('createCalendarObject')->with(666, '63f0dd6c-39d5-44be-9d34-34e7a7441fc2.ics', 'BEGIN:VCARD');
+		$cardDav->expects($this->once())->method('updateShares')->with($this->anything(), [
+			['href' => 'principal:principals/groups/users', 'readOnly' => false],
+			['href' => 'principal:principals/users/adam', 'readOnly' => true]
+		]);
 		/** @var ILogger $logger */
 		$logger = $this->getMockBuilder('\OCP\ILogger')->disableOriginalConstructor()->getMock();
 
@@ -50,7 +57,7 @@ class MigrateCalendarTest extends TestCase {
 		$adapter = $this->getMockBuilder('\OCA\Dav\Migration\CalendarAdapter')
 			->disableOriginalConstructor()
 			->getMock();
-		$adapter->method('foreachCalendar')->willReturnCallback(function ($user, \Closure $callBack) {
+		$adapter->expects($this->any())->method('foreachCalendar')->willReturnCallback(function ($user, \Closure $callBack) {
 			$callBack([
 				// calendarorder | calendarcolor | timezone | components
 				'id' => 0,
@@ -65,14 +72,14 @@ class MigrateCalendarTest extends TestCase {
 				'components' => 'VEVENT,VTODO,VJOURNAL'
 			]);
 		});
-		$adapter->method('foreachCalendarObject')->willReturnCallback(function ($addressBookId, \Closure $callBack) use ($calData) {
+		$adapter->expects($this->any())->method('foreachCalendarObject')->willReturnCallback(function ($addressBookId, \Closure $callBack) use ($calData) {
 			$callBack([
 				'userid' => $addressBookId,
 				'uri' => '63f0dd6c-39d5-44be-9d34-34e7a7441fc2.ics',
 				'calendardata' => $calData
 			]);
 		});
-		$adapter->method('getShares')->willReturn($shares);
+		$adapter->expects($this->any())->method('getShares')->willReturn($shares);
 		return $adapter;
 	}
 }