Merge pull request #21195 from owncloud/test-carddav-sharing-plugin

Fix carddav sharing plugin + adding unit tests

5 files changed, 167 insertions, 64 deletions

diff --git a/apps/dav/lib/carddav/carddavbackend.php b/apps/dav/lib/carddav/carddavbackend.php
index 742d29e92c1..95175b20d1b 100644
--- a/apps/dav/lib/carddav/carddavbackend.php
+++ b/apps/dav/lib/carddav/carddavbackend.php
@@ -781,7 +781,7 @@ class CardDavBackend implements BackendInterface, SyncSupport {
 		}
 
 		// remove the share if it already exists
-		$this->unshare($addressBookUri, $element);
+		$this->unshare($addressBookUri, $element['href']);
 
 		$query = $this->db->getQueryBuilder();
 		$query->insert('dav_shares')
@@ -800,8 +800,7 @@ class CardDavBackend implements BackendInterface, SyncSupport {
 	 * @param string $element
 	 */
 	private function unshare($addressBookUri, $element) {
-		$user = $element['href'];
-		$parts = explode(':', $user, 2);
+		$parts = explode(':', $element, 2);
 		if ($parts[0] !== 'principal') {
 			return;
 		}
diff --git a/apps/dav/lib/carddav/sharing/plugin.php b/apps/dav/lib/carddav/sharing/plugin.php
index 99c6f8f912c..fd415b4566b 100644
--- a/apps/dav/lib/carddav/sharing/plugin.php
+++ b/apps/dav/lib/carddav/sharing/plugin.php
@@ -9,11 +9,24 @@ use Sabre\DAV\Exception\NotFound;
 use Sabre\DAV\Server;
 use Sabre\DAV\ServerPlugin;
 use Sabre\DAV\XMLUtil;
+use Sabre\DAVACL\IACL;
 use Sabre\HTTP\RequestInterface;
 use Sabre\HTTP\ResponseInterface;
 
 class Plugin extends ServerPlugin {
 
+	/** @var Auth */
+	private $auth;
+
+	/** @var IRequest */
+	private $request;
+
+	/**
+	 * Plugin constructor.
+	 *
+	 * @param Auth $authBackEnd
+	 * @param IRequest $request
+	 */
 	public function __construct(Auth $authBackEnd, IRequest $request) {
 		$this->auth = $authBackEnd;
 		$this->request = $request;
@@ -68,6 +81,7 @@ class Plugin extends ServerPlugin {
 	function initialize(Server $server) {
 		$this->server = $server;
 		$server->resourceTypeMapping['OCA\\DAV\CardDAV\\ISharedAddressbook'] = '{' . \Sabre\CardDAV\Plugin::NS_CARDDAV . '}shared';
+		$this->server->xml->elementMap['{' . \Sabre\CardDAV\Plugin::NS_CARDDAV . '}share'] = 'OCA\\DAV\\CardDAV\\Sharing\\Xml\\ShareRequest';
 
 		$this->server->on('method:POST', [$this, 'httpPost']);
 	}
@@ -109,9 +123,7 @@ class Plugin extends ServerPlugin {
 		// re-populated the request body with the existing data.
 		$request->setBody($requestBody);
 
-		$dom = XMLUtil::loadDOMDocument($requestBody);
-
-		$documentType = XMLUtil::toClarkNotation($dom->firstChild);
+		$message = $this->server->xml->parse($requestBody, $request->getUrl(), $documentType);
 
 		switch ($documentType) {
 
@@ -124,19 +136,18 @@ class Plugin extends ServerPlugin {
 					return;
 				}
 
-				$this->server->transactionType = 'post-calendar-share';
+				$this->server->transactionType = 'post-oc-addressbook-share';
 
 				// Getting ACL info
 				$acl = $this->server->getPlugin('acl');
 
 				// If there's no ACL support, we allow everything
 				if ($acl) {
+					/** @var \Sabre\DAVACL\Plugin $acl */
 					$acl->checkPrivileges($path, '{DAV:}write');
 				}
 
-				$mutations = $this->parseShareRequest($dom);
-
-				$node->updateShares($mutations[0], $mutations[1]);
+				$node->updateShares($message->set, $message->remove);
 
 				$response->setStatus(200);
 				// Adding this because sending a response body may cause issues,
@@ -148,59 +159,6 @@ class Plugin extends ServerPlugin {
 		}
 	}
 
-	/**
-	 * Parses the 'share' POST request.
-	 *
-	 * This method returns an array, containing two arrays.
-	 * The first array is a list of new sharees. Every element is a struct
-	 * containing a:
-	 *   * href element. (usually a mailto: address)
-	 *   * commonName element (often a first and lastname, but can also be
-	 *     false)
-	 *   * readOnly (true or false)
-	 *   * summary (A description of the share, can also be false)
-	 *
-	 * The second array is a list of sharees that are to be removed. This is
-	 * just a simple array with 'hrefs'.
-	 *
-	 * @param \DOMDocument $dom
-	 * @return array
-	 */
-	function parseShareRequest(\DOMDocument $dom) {
-
-		$xpath = new \DOMXPath($dom);
-		$xpath->registerNamespace('cs', \Sabre\CardDAV\Plugin::NS_CARDDAV);
-		$xpath->registerNamespace('d', 'urn:DAV');
-
-		$set = [];
-		$elems = $xpath->query('cs:set');
-
-		for ($i = 0; $i < $elems->length; $i++) {
-
-			$xset = $elems->item($i);
-			$set[] = [
-				'href' => $xpath->evaluate('string(d:href)', $xset),
-				'commonName' => $xpath->evaluate('string(cs:common-name)', $xset),
-				'summary' => $xpath->evaluate('string(cs:summary)', $xset),
-				'readOnly' => $xpath->evaluate('boolean(cs:read)', $xset) !== false
-			];
-
-		}
-
-		$remove = [];
-		$elems = $xpath->query('cs:remove');
-
-		for ($i = 0; $i < $elems->length; $i++) {
-
-			$xremove = $elems->item($i);
-			$remove[] = $xpath->evaluate('string(d:href)', $xremove);
-
-		}
-
-		return [$set, $remove];
-
-	}
-
 	private function protectAgainstCSRF() {
 		$user = $this->auth->getCurrentUser();
 		if ($this->auth->isDavAuthenticated($user)) {
diff --git a/apps/dav/lib/carddav/sharing/xml/sharerequest.php b/apps/dav/lib/carddav/sharing/xml/sharerequest.php
new file mode 100644
index 00000000000..175c5ffc306
--- /dev/null
+++ b/apps/dav/lib/carddav/sharing/xml/sharerequest.php
@@ -0,0 +1,65 @@
+<?php
+
+namespace OCA\DAV\CardDAV\Sharing\Xml;
+
+use Sabre\Xml\Reader;
+use Sabre\Xml\XmlDeserializable;
+
+class ShareRequest implements XmlDeserializable {
+
+    public $set = [];
+
+    public $remove = [];
+
+    /**
+     * Constructor
+     *
+     * @param array $set
+     * @param array $remove
+     */
+    function __construct(array $set, array $remove) {
+
+        $this->set = $set;
+        $this->remove = $remove;
+
+    }
+
+    static function xmlDeserialize(Reader $reader) {
+
+        $elems = $reader->parseInnerTree([
+            '{' . \Sabre\CardDAV\Plugin::NS_CARDDAV. '}set'    => 'Sabre\\Xml\\Element\\KeyValue',
+            '{' . \Sabre\CardDAV\Plugin::NS_CARDDAV . '}remove' => 'Sabre\\Xml\\Element\\KeyValue',
+        ]);
+
+        $set = [];
+        $remove = [];
+
+        foreach ($elems as $elem) {
+            switch ($elem['name']) {
+
+                case '{' . \Sabre\CardDAV\Plugin::NS_CARDDAV . '}set' :
+                    $sharee = $elem['value'];
+
+                    $sumElem = '{' . \Sabre\CardDAV\Plugin::NS_CARDDAV . '}summary';
+                    $commonName = '{' . \Sabre\CardDAV\Plugin::NS_CARDDAV . '}common-name';
+
+                    $set[] = [
+                        'href'       => $sharee['{DAV:}href'],
+                        'commonName' => isset($sharee[$commonName]) ? $sharee[$commonName] : null,
+                        'summary'    => isset($sharee[$sumElem]) ? $sharee[$sumElem] : null,
+                        'readOnly'   => !array_key_exists('{' . \Sabre\CardDAV\Plugin::NS_CARDDAV . '}read-write', $sharee),
+                    ];
+                    break;
+
+                case '{' . \Sabre\CardDAV\Plugin::NS_CARDDAV . '}remove' :
+                    $remove[] = $elem['value']['{DAV:}href'];
+                    break;
+
+            }
+        }
+
+        return new self($set, $remove);
+
+    }
+
+}
diff --git a/apps/dav/tests/unit/carddav/carddavbackendtest.php b/apps/dav/tests/unit/carddav/carddavbackendtest.php
index 56d04a8cd44..fe01aa65cca 100644
--- a/apps/dav/tests/unit/carddav/carddavbackendtest.php
+++ b/apps/dav/tests/unit/carddav/carddavbackendtest.php
@@ -263,7 +263,7 @@ class CardDavBackendTest extends TestCase {
 		$books = $this->backend->getAddressBooksForUser('principals/best-friend');
 		$this->assertEquals(1, count($books));
 
-		$this->backend->updateShares('Example', [], [['href' => 'principal:principals/best-friend']]);
+		$this->backend->updateShares('Example', [], ['principal:principals/best-friend']);
 
 		$shares = $this->backend->getShares('Example');
 		$this->assertEquals(0, count($shares));
diff --git a/apps/dav/tests/unit/carddav/sharing/plugintest.php b/apps/dav/tests/unit/carddav/sharing/plugintest.php
new file mode 100644
index 00000000000..e9532acbe3b
--- /dev/null
+++ b/apps/dav/tests/unit/carddav/sharing/plugintest.php
@@ -0,0 +1,81 @@
+<?php
+/**
+ * @author Thomas Müller <thomas.mueller@tmit.eu>
+ *
+ * @copyright Copyright (c) 2015, 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\DAV\Tests\Unit\CardDAV;
+
+
+use OCA\DAV\CardDAV\Sharing\IShareableAddressBook;
+use OCA\DAV\CardDAV\Sharing\Plugin;
+use OCA\DAV\Connector\Sabre\Auth;
+use OCP\IRequest;
+use Sabre\DAV\Server;
+use Sabre\DAV\SimpleCollection;
+use Sabre\HTTP\Request;
+use Sabre\HTTP\Response;
+use Test\TestCase;
+
+class PluginTest extends TestCase {
+
+	/** @var Plugin */
+	private $plugin;
+	/** @var Server */
+	private $server;
+	/** @var IShareableAddressBook | \PHPUnit_Framework_MockObject_MockObject */
+	private $book;
+
+	public function setUp() {
+		parent::setUp();
+		
+		/** @var Auth | \PHPUnit_Framework_MockObject_MockObject $authBackend */
+		$authBackend = $this->getMockBuilder('OCA\DAV\Connector\Sabre\Auth')->disableOriginalConstructor()->getMock();
+		$authBackend->method('isDavAuthenticated')->willReturn(true);
+
+		/** @var IRequest $request */
+		$request = $this->getMockBuilder('OCP\IRequest')->disableOriginalConstructor()->getMock();
+		$this->plugin = new Plugin($authBackend, $request);
+
+		$root = new SimpleCollection('root');
+		$this->server = new \Sabre\DAV\Server($root);
+		/** @var SimpleCollection $node */
+		$this->book = $this->getMockBuilder('OCA\DAV\CardDAV\Sharing\IShareableAddressBook')->disableOriginalConstructor()->getMock();
+		$this->book->method('getName')->willReturn('addressbook1.vcf');
+		$root->addChild($this->book);
+		$this->plugin->initialize($this->server);
+	}
+
+	public function testSharing() {
+
+		$this->book->expects($this->once())->method('updateShares')->with([[
+				'href' => 'principal:principals/admin',
+				'commonName' => null,
+				'summary' => null,
+				'readOnly' => false
+		]], ['mailto:wilfredo@example.com']);
+
+		// setup request
+		$request = new Request();
+		$request->addHeader('Content-Type', 'application/xml');
+		$request->setUrl('addressbook1.vcf');
+		$request->setBody('<?xml version="1.0" encoding="utf-8" ?><CS:share xmlns:D="DAV:" xmlns:CS="urn:ietf:params:xml:ns:carddav"><CS:set><D:href>principal:principals/admin</D:href><CS:read-write/></CS:set> <CS:remove><D:href>mailto:wilfredo@example.com</D:href></CS:remove></CS:share>');
+		$response = new Response();
+		$this->plugin->httpPost($request, $response);
+	}
+}