From c25a7cc4daf486b7ad8e50a5209acda061734d6c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Thomas=20M=C3=BCller?= <thomas.mueller@tmit.eu>
Date: Tue, 24 Nov 2015 11:15:31 +0100
Subject: [PATCH] Users are available under it's own principal resource named
 'principals/users' this will allow us to introduce e.g. groups as principals
 (one day) and system specific principals (needed for federation)

---
 apps/dav/command/createaddressbook.php        |  2 +-
 apps/dav/command/createcalendar.php           |  2 +-
 apps/dav/lib/connector/sabre/auth.php         |  1 +
 apps/dav/lib/connector/sabre/principal.php    | 68 +++++++++++--------
 apps/dav/lib/rootcollection.php               |  6 +-
 apps/dav/lib/server.php                       |  6 +-
 apps/dav/tests/unit/connector/sabre/auth.php  |  4 +-
 .../tests/unit/connector/sabre/principal.php  | 58 ++++++++--------
 8 files changed, 82 insertions(+), 65 deletions(-)

diff --git a/apps/dav/command/createaddressbook.php b/apps/dav/command/createaddressbook.php
index 371ea44121d..ea89e7aa0a8 100644
--- a/apps/dav/command/createaddressbook.php
+++ b/apps/dav/command/createaddressbook.php
@@ -58,6 +58,6 @@ class CreateAddressBook extends Command {
 
 		$name = $input->getArgument('name');
 		$carddav = new CardDavBackend($this->dbConnection, $principalBackend);
-		$carddav->createAddressBook("principals/$user", $name, []);
+		$carddav->createAddressBook("principals/users/$user", $name, []);
 	}
 }
diff --git a/apps/dav/command/createcalendar.php b/apps/dav/command/createcalendar.php
index da4f248e51d..5e7b17dae66 100644
--- a/apps/dav/command/createcalendar.php
+++ b/apps/dav/command/createcalendar.php
@@ -47,6 +47,6 @@ class CreateCalendar extends Command {
 		}
 		$name = $input->getArgument('name');
 		$caldav = new CalDavBackend($this->dbConnection);
-		$caldav->createCalendar("principals/$user", $name, []);
+		$caldav->createCalendar("principals/users/$user", $name, []);
 	}
 }
diff --git a/apps/dav/lib/connector/sabre/auth.php b/apps/dav/lib/connector/sabre/auth.php
index 655152a2cc1..803db78ecd7 100644
--- a/apps/dav/lib/connector/sabre/auth.php
+++ b/apps/dav/lib/connector/sabre/auth.php
@@ -54,6 +54,7 @@ class Auth extends AbstractBasic {
 								IUserSession $userSession) {
 		$this->session = $session;
 		$this->userSession = $userSession;
+		$this->principalPrefix = 'principals/users/';
 	}
 
 	/**
diff --git a/apps/dav/lib/connector/sabre/principal.php b/apps/dav/lib/connector/sabre/principal.php
index 7fb14c031f9..cd3b14b6841 100644
--- a/apps/dav/lib/connector/sabre/principal.php
+++ b/apps/dav/lib/connector/sabre/principal.php
@@ -30,9 +30,11 @@
 
 namespace OCA\DAV\Connector\Sabre;
 
+use OCP\IUser;
 use OCP\IUserManager;
 use OCP\IConfig;
 use \Sabre\DAV\PropPatch;
+use Sabre\HTTP\URLUtil;
 
 class Principal implements \Sabre\DAVACL\PrincipalBackend\BackendInterface {
 	/** @var IConfig */
@@ -66,20 +68,9 @@ class Principal implements \Sabre\DAVACL\PrincipalBackend\BackendInterface {
 	public function getPrincipalsByPrefix($prefixPath) {
 		$principals = [];
 
-		if ($prefixPath === 'principals') {
+		if ($prefixPath === 'principals/users') {
 			foreach($this->userManager->search('') as $user) {
-
-				$principal = [
-					'uri' => 'principals/' . $user->getUID(),
-					'{DAV:}displayname' => $user->getUID(),
-				];
-
-				$email = $this->config->getUserValue($user->getUID(), 'settings', 'email');
-				if(!empty($email)) {
-					$principal['{http://sabredav.org/ns}email-address'] = $email;
-				}
-
-				$principals[] = $principal;
+				$principals[] = $this->userToPrincipal($user);
 			}
 		}
 
@@ -95,21 +86,18 @@ class Principal implements \Sabre\DAVACL\PrincipalBackend\BackendInterface {
 	 * @return array
 	 */
 	public function getPrincipalByPath($path) {
-		list($prefix, $name) = explode('/', $path);
+		$elements = explode('/', $path);
+		if ($elements[0] !== 'principals') {
+			return null;
+		}
+		if ($elements[1] !== 'users') {
+			return null;
+		}
+		$name = $elements[2];
 		$user = $this->userManager->get($name);
 
-		if ($prefix === 'principals' && !is_null($user)) {
-			$principal = [
-				'uri' => 'principals/' . $user->getUID(),
-				'{DAV:}displayname' => $user->getUID(),
-			];
-
-			$email = $this->config->getUserValue($user->getUID(), 'settings', 'email');
-			if($email) {
-				$principal['{http://sabredav.org/ns}email-address'] = $email;
-			}
-
-			return $principal;
+		if (!is_null($user)) {
+			return $this->userToPrincipal($user);
 		}
 
 		return null;
@@ -140,10 +128,10 @@ class Principal implements \Sabre\DAVACL\PrincipalBackend\BackendInterface {
 	 * @throws \Sabre\DAV\Exception
 	 */
 	public function getGroupMembership($principal) {
-		list($prefix, $name) = \Sabre\HTTP\URLUtil::splitPath($principal);
+		list($prefix, $name) = URLUtil::splitPath($principal);
 
 		$group_membership = array();
-		if ($prefix === 'principals') {
+		if ($prefix === 'principals/users') {
 			$principal = $this->getPrincipalByPath($principal);
 			if (!$principal) {
 				throw new \Sabre\DAV\Exception('Principal not found');
@@ -151,8 +139,8 @@ class Principal implements \Sabre\DAVACL\PrincipalBackend\BackendInterface {
 
 			// TODO: for now the user principal has only its own groups
 			return array(
-				'principals/'.$name.'/calendar-proxy-read',
-				'principals/'.$name.'/calendar-proxy-write',
+				'principals/users/'.$name.'/calendar-proxy-read',
+				'principals/users/'.$name.'/calendar-proxy-write',
 				// The addressbook groups are not supported in Sabre,
 				// see http://groups.google.com/group/sabredav-discuss/browse_thread/thread/ef2fa9759d55f8c#msg_5720afc11602e753
 				//'principals/'.$name.'/addressbook-proxy-read',
@@ -202,4 +190,24 @@ class Principal implements \Sabre\DAVACL\PrincipalBackend\BackendInterface {
 	function findByUri($uri, $principalPrefix) {
 		return '';
 	}
+
+	/**
+	 * @param IUser $user
+	 * @return array
+	 */
+	protected function userToPrincipal($user) {
+		$userId = $user->getUID();
+		$displayName = $user->getDisplayName();
+		$principal = [
+				'uri' => "principals/users/$userId",
+				'{DAV:}displayname' => is_null($displayName) ? $userId : $displayName,
+		];
+
+		$email = $this->config->getUserValue($user->getUID(), 'settings', 'email');
+		if (!empty($email)) {
+			$principal['{http://sabredav.org/ns}email-address'] = $email;
+			return $principal;
+		}
+		return $principal;
+	}
 }
diff --git a/apps/dav/lib/rootcollection.php b/apps/dav/lib/rootcollection.php
index 672e0a98684..c0a37a1de9d 100644
--- a/apps/dav/lib/rootcollection.php
+++ b/apps/dav/lib/rootcollection.php
@@ -23,9 +23,9 @@ class RootCollection extends SimpleCollection {
 		$disableListing = !$config->getSystemValue('debug', false);
 
 		// setup the first level of the dav tree
-		$principalCollection = new Collection($principalBackend);
+		$principalCollection = new Collection($principalBackend, 'principals/users');
 		$principalCollection->disableListing = $disableListing;
-		$filesCollection = new Files\RootCollection($principalBackend);
+		$filesCollection = new Files\RootCollection($principalBackend, 'principals/users');
 		$filesCollection->disableListing = $disableListing;
 		$caldavBackend = new CalDavBackend($db);
 		$calendarRoot = new CalendarRoot($principalBackend, $caldavBackend);
@@ -37,7 +37,7 @@ class RootCollection extends SimpleCollection {
 		$addressBookRoot->disableListing = $disableListing;
 
 		$children = [
-				$principalCollection,
+				new SimpleCollection('principals', [$principalCollection]),
 				$filesCollection,
 				$calendarRoot,
 				$addressBookRoot,
diff --git a/apps/dav/lib/server.php b/apps/dav/lib/server.php
index 587c0091e23..ffdb917085e 100644
--- a/apps/dav/lib/server.php
+++ b/apps/dav/lib/server.php
@@ -41,9 +41,13 @@ class Server {
 		$this->server->addPlugin(new \OCA\DAV\Connector\Sabre\ListenerPlugin($dispatcher));
 		$this->server->addPlugin(new \Sabre\DAV\Sync\Plugin());
 
+		// acl
+		$acl = new \Sabre\DAVACL\Plugin();
+		$acl->defaultUsernamePath = 'principals/users';
+		$this->server->addPlugin($acl);
+
 		// calendar plugins
 		$this->server->addPlugin(new \Sabre\CalDAV\Plugin());
-		$this->server->addPlugin(new \Sabre\DAVACL\Plugin());
 		$this->server->addPlugin(new \Sabre\CalDAV\ICSExportPlugin());
 		$senderEmail = \OCP\Util::getDefaultEmailAddress('no-reply');
 		$this->server->addPlugin(new \Sabre\CalDAV\Schedule\Plugin());
diff --git a/apps/dav/tests/unit/connector/sabre/auth.php b/apps/dav/tests/unit/connector/sabre/auth.php
index 595bd441617..47dd237b761 100644
--- a/apps/dav/tests/unit/connector/sabre/auth.php
+++ b/apps/dav/tests/unit/connector/sabre/auth.php
@@ -279,7 +279,7 @@ class Auth extends TestCase {
 			->method('close');
 
 		$response = $this->auth->check($request, $response);
-		$this->assertEquals([true, 'principals/MyWrongDavUser'], $response);
+		$this->assertEquals([true, 'principals/users/MyWrongDavUser'], $response);
 	}
 
 	public function testAuthenticateNoBasicAuthenticateHeadersProvided() {
@@ -353,7 +353,7 @@ class Auth extends TestCase {
 			->method('getUser')
 			->will($this->returnValue($user));
 		$response = $this->auth->check($server->httpRequest, $server->httpResponse);
-		$this->assertEquals([true, 'principals/username'], $response);
+		$this->assertEquals([true, 'principals/users/username'], $response);
 	}
 
 	public function testAuthenticateInvalidCredentials() {
diff --git a/apps/dav/tests/unit/connector/sabre/principal.php b/apps/dav/tests/unit/connector/sabre/principal.php
index 2fbab124fb7..1d7721c546d 100644
--- a/apps/dav/tests/unit/connector/sabre/principal.php
+++ b/apps/dav/tests/unit/connector/sabre/principal.php
@@ -41,13 +41,17 @@ class Principal extends \Test\TestCase {
 		$fooUser = $this->getMockBuilder('\OC\User\User')
 			->disableOriginalConstructor()->getMock();
 		$fooUser
-			->expects($this->exactly(3))
-			->method('getUID')
-			->will($this->returnValue('foo'));
+				->expects($this->exactly(2))
+				->method('getUID')
+				->will($this->returnValue('foo'));
+		$fooUser
+				->expects($this->exactly(1))
+				->method('getDisplayName')
+				->will($this->returnValue('Dr. Foo-Bar'));
 		$barUser = $this->getMockBuilder('\OC\User\User')
 			->disableOriginalConstructor()->getMock();
 		$barUser
-			->expects($this->exactly(3))
+			->expects($this->exactly(2))
 			->method('getUID')
 			->will($this->returnValue('bar'));
 		$this->userManager
@@ -68,16 +72,16 @@ class Principal extends \Test\TestCase {
 
 		$expectedResponse = [
 			0 => [
-				'uri' => 'principals/foo',
-				'{DAV:}displayname' => 'foo'
+				'uri' => 'principals/users/foo',
+				'{DAV:}displayname' => 'Dr. Foo-Bar'
 			],
 			1 => [
-				'uri' => 'principals/bar',
+				'uri' => 'principals/users/bar',
 				'{DAV:}displayname' => 'bar',
 				'{http://sabredav.org/ns}email-address' => 'bar@owncloud.org'
 			]
 		];
-		$response = $this->connector->getPrincipalsByPrefix('principals');
+		$response = $this->connector->getPrincipalsByPrefix('principals/users');
 		$this->assertSame($expectedResponse, $response);
 	}
 
@@ -88,7 +92,7 @@ class Principal extends \Test\TestCase {
 			->with('')
 			->will($this->returnValue([]));
 
-		$response = $this->connector->getPrincipalsByPrefix('principals');
+		$response = $this->connector->getPrincipalsByPrefix('principals/users');
 		$this->assertSame([], $response);
 	}
 
@@ -96,7 +100,7 @@ class Principal extends \Test\TestCase {
 		$fooUser = $this->getMockBuilder('\OC\User\User')
 			->disableOriginalConstructor()->getMock();
 		$fooUser
-			->expects($this->exactly(3))
+			->expects($this->exactly(2))
 			->method('getUID')
 			->will($this->returnValue('foo'));
 		$this->userManager
@@ -111,10 +115,10 @@ class Principal extends \Test\TestCase {
 			->will($this->returnValue(''));
 
 		$expectedResponse = [
-			'uri' => 'principals/foo',
+			'uri' => 'principals/users/foo',
 			'{DAV:}displayname' => 'foo'
 		];
-		$response = $this->connector->getPrincipalByPath('principals/foo');
+		$response = $this->connector->getPrincipalByPath('principals/users/foo');
 		$this->assertSame($expectedResponse, $response);
 	}
 
@@ -122,7 +126,7 @@ class Principal extends \Test\TestCase {
 		$fooUser = $this->getMockBuilder('\OC\User\User')
 			->disableOriginalConstructor()->getMock();
 		$fooUser
-			->expects($this->exactly(3))
+			->expects($this->exactly(2))
 			->method('getUID')
 			->will($this->returnValue('foo'));
 		$this->userManager
@@ -137,11 +141,11 @@ class Principal extends \Test\TestCase {
 			->will($this->returnValue('foo@owncloud.org'));
 
 		$expectedResponse = [
-			'uri' => 'principals/foo',
+			'uri' => 'principals/users/foo',
 			'{DAV:}displayname' => 'foo',
 			'{http://sabredav.org/ns}email-address' => 'foo@owncloud.org'
 		];
-		$response = $this->connector->getPrincipalByPath('principals/foo');
+		$response = $this->connector->getPrincipalByPath('principals/users/foo');
 		$this->assertSame($expectedResponse, $response);
 	}
 
@@ -152,7 +156,7 @@ class Principal extends \Test\TestCase {
 			->with('foo')
 			->will($this->returnValue(null));
 
-		$response = $this->connector->getPrincipalByPath('principals/foo');
+		$response = $this->connector->getPrincipalByPath('principals/users/foo');
 		$this->assertSame(null, $response);
 	}
 
@@ -160,7 +164,7 @@ class Principal extends \Test\TestCase {
 		$fooUser = $this->getMockBuilder('\OC\User\User')
 			->disableOriginalConstructor()->getMock();
 		$fooUser
-			->expects($this->exactly(3))
+			->expects($this->exactly(2))
 			->method('getUID')
 			->will($this->returnValue('foo'));
 		$this->userManager
@@ -174,8 +178,8 @@ class Principal extends \Test\TestCase {
 			->with('foo', 'settings', 'email')
 			->will($this->returnValue('foo@owncloud.org'));
 
-		$response = $this->connector->getGroupMemberSet('principals/foo');
-		$this->assertSame(['principals/foo'], $response);
+		$response = $this->connector->getGroupMemberSet('principals/users/foo');
+		$this->assertSame(['principals/users/foo'], $response);
 	}
 
 	/**
@@ -189,14 +193,14 @@ class Principal extends \Test\TestCase {
 			->with('foo')
 			->will($this->returnValue(null));
 
-		$this->connector->getGroupMemberSet('principals/foo');
+		$this->connector->getGroupMemberSet('principals/users/foo');
 	}
 
 	public function testGetGroupMembership() {
 		$fooUser = $this->getMockBuilder('\OC\User\User')
 			->disableOriginalConstructor()->getMock();
 		$fooUser
-			->expects($this->exactly(3))
+			->expects($this->exactly(2))
 			->method('getUID')
 			->will($this->returnValue('foo'));
 		$this->userManager
@@ -211,10 +215,10 @@ class Principal extends \Test\TestCase {
 			->will($this->returnValue('foo@owncloud.org'));
 
 		$expectedResponse = [
-			'principals/foo/calendar-proxy-read',
-			'principals/foo/calendar-proxy-write'
+			'principals/users/foo/calendar-proxy-read',
+			'principals/users/foo/calendar-proxy-write'
 		];
-		$response = $this->connector->getGroupMembership('principals/foo');
+		$response = $this->connector->getGroupMembership('principals/users/foo');
 		$this->assertSame($expectedResponse, $response);
 	}
 
@@ -229,7 +233,7 @@ class Principal extends \Test\TestCase {
 			->with('foo')
 			->will($this->returnValue(null));
 
-		$this->connector->getGroupMembership('principals/foo');
+		$this->connector->getGroupMembership('principals/users/foo');
 	}
 
 	/**
@@ -237,7 +241,7 @@ class Principal extends \Test\TestCase {
 	 * @expectedExceptionMessage Setting members of the group is not supported yet
 	 */
 	public function testSetGroupMembership() {
-		$this->connector->setGroupMemberSet('principals/foo', ['foo']);
+		$this->connector->setGroupMemberSet('principals/users/foo', ['foo']);
 	}
 
 	public function testUpdatePrincipal() {
@@ -245,6 +249,6 @@ class Principal extends \Test\TestCase {
 	}
 
 	public function testSearchPrincipals() {
-		$this->assertSame([], $this->connector->searchPrincipals('principals', []));
+		$this->assertSame([], $this->connector->searchPrincipals('principals/users', []));
 	}
 }
-- 
GitLab