From ae36c01b959d1cbdaeabb46d30cb416aae428a88 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Thomas=20M=C3=BCller?= Date: Fri, 20 Nov 2015 13:35:23 +0100 Subject: [PATCH] Adjust sabre changes in core --- apps/dav/lib/caldav/caldavbackend.php | 4 +- apps/dav/lib/carddav/addressbook.php | 6 +- apps/dav/lib/carddav/useraddressbooks.php | 2 +- apps/dav/lib/connector/sabre/auth.php | 39 ++--- .../lib/connector/sabre/fakelockerplugin.php | 18 +- apps/dav/lib/connector/sabre/taglist.php | 158 ++++++++++-------- .../tests/unit/caldav/caldavbackendtest.php | 4 +- .../connector/sabre/FakeLockerPluginTest.php | 12 +- apps/dav/tests/unit/connector/sabre/auth.php | 42 +++-- .../unit/connector/sabre/requesttest/auth.php | 62 +++++-- lib/private/files/storage/dav.php | 9 +- 11 files changed, 198 insertions(+), 158 deletions(-) diff --git a/apps/dav/lib/caldav/caldavbackend.php b/apps/dav/lib/caldav/caldavbackend.php index 99338650793..d912f209d46 100644 --- a/apps/dav/lib/caldav/caldavbackend.php +++ b/apps/dav/lib/caldav/caldavbackend.php @@ -26,8 +26,8 @@ use Sabre\CalDAV\Backend\SchedulingSupport; use Sabre\CalDAV\Backend\SubscriptionSupport; use Sabre\CalDAV\Backend\SyncSupport; use Sabre\CalDAV\Plugin; -use Sabre\CalDAV\Property\ScheduleCalendarTransp; -use Sabre\CalDAV\Property\SupportedCalendarComponentSet; +use Sabre\CalDAV\Xml\Property\ScheduleCalendarTransp; +use Sabre\CalDAV\Xml\Property\SupportedCalendarComponentSet; use Sabre\DAV; use Sabre\DAV\Exception\Forbidden; use Sabre\VObject\DateTimeParser; diff --git a/apps/dav/lib/carddav/addressbook.php b/apps/dav/lib/carddav/addressbook.php index e50f6f4adf6..eff1ad321e5 100644 --- a/apps/dav/lib/carddav/addressbook.php +++ b/apps/dav/lib/carddav/addressbook.php @@ -3,13 +3,9 @@ namespace OCA\DAV\CardDAV; use OCA\DAV\CardDAV\Sharing\IShareableAddressBook; -use OCP\IUserManager; class AddressBook extends \Sabre\CardDAV\AddressBook implements IShareableAddressBook { - /** @var IUserManager */ - private $userManager; - public function __construct(CardDavBackend $carddavBackend, array $addressBookInfo) { parent::__construct($carddavBackend, $addressBookInfo); } @@ -55,4 +51,4 @@ class AddressBook extends \Sabre\CardDAV\AddressBook implements IShareableAddres $carddavBackend = $this->carddavBackend; $carddavBackend->getShares($this->getName()); } -} \ No newline at end of file +} diff --git a/apps/dav/lib/carddav/useraddressbooks.php b/apps/dav/lib/carddav/useraddressbooks.php index adbb0292fa7..5f618a0ece3 100644 --- a/apps/dav/lib/carddav/useraddressbooks.php +++ b/apps/dav/lib/carddav/useraddressbooks.php @@ -2,7 +2,7 @@ namespace OCA\DAV\CardDAV; -class UserAddressBooks extends \Sabre\CardDAV\UserAddressBooks { +class UserAddressBooks extends \Sabre\CardDAV\AddressBookHome { /** * Returns a list of addressbooks diff --git a/apps/dav/lib/connector/sabre/auth.php b/apps/dav/lib/connector/sabre/auth.php index 27f6704ba2c..655152a2cc1 100644 --- a/apps/dav/lib/connector/sabre/auth.php +++ b/apps/dav/lib/connector/sabre/auth.php @@ -35,6 +35,8 @@ use OCP\IUserSession; use Sabre\DAV\Auth\Backend\AbstractBasic; use Sabre\DAV\Exception\NotAuthenticated; use Sabre\DAV\Exception\ServiceUnavailable; +use Sabre\HTTP\RequestInterface; +use Sabre\HTTP\ResponseInterface; class Auth extends AbstractBasic { const DAV_AUTHENTICATED = 'AUTHENTICATED_TO_DAV_BACKEND'; @@ -122,22 +124,15 @@ class Auth extends AbstractBasic { } /** - * Override function here. We want to cache authentication cookies - * in the syncing client to avoid HTTP-401 roundtrips. - * If the sync client supplies the cookies, then OC_User::isLoggedIn() - * will return true and we can see this WebDAV request as already authenticated, - * even if there are no HTTP Basic Auth headers. - * In other case, just fallback to the parent implementation. - * - * @param \Sabre\DAV\Server $server - * @param string $realm - * @return bool - * @throws ServiceUnavailable + * @param RequestInterface $request + * @param ResponseInterface $response + * @return array * @throws NotAuthenticated + * @throws ServiceUnavailable */ - public function authenticate(\Sabre\DAV\Server $server, $realm) { + function check(RequestInterface $request, ResponseInterface $response) { try { - $result = $this->auth($server, $realm); + $result = $this->auth($request, $response); return $result; } catch (NotAuthenticated $e) { throw $e; @@ -149,11 +144,11 @@ class Auth extends AbstractBasic { } /** - * @param \Sabre\DAV\Server $server - * @param string $realm - * @return bool + * @param RequestInterface $request + * @param ResponseInterface $response + * @return array */ - private function auth(\Sabre\DAV\Server $server, $realm) { + private function auth(RequestInterface $request, ResponseInterface $response) { if (\OC_User::handleApacheAuth() || ($this->userSession->isLoggedIn() && is_null($this->session->get(self::DAV_AUTHENTICATED))) ) { @@ -161,16 +156,16 @@ class Auth extends AbstractBasic { \OC_Util::setupFS($user); $this->currentUser = $user; $this->session->close(); - return true; + return [true, $this->principalPrefix . $user]; } - if ($server->httpRequest->getHeader('X-Requested-With') === 'XMLHttpRequest') { + if ($request->getHeader('X-Requested-With') === 'XMLHttpRequest') { // do not re-authenticate over ajax, use dummy auth name to prevent browser popup - $server->httpResponse->addHeader('WWW-Authenticate','DummyBasic realm="' . $realm . '"'); - $server->httpResponse->setStatus(401); + $response->addHeader('WWW-Authenticate','DummyBasic realm="' . $this->realm . '"'); + $response->setStatus(401); throw new \Sabre\DAV\Exception\NotAuthenticated('Cannot authenticate over ajax calls'); } - return parent::authenticate($server, $realm); + return parent::check($request, $response); } } diff --git a/apps/dav/lib/connector/sabre/fakelockerplugin.php b/apps/dav/lib/connector/sabre/fakelockerplugin.php index b9d1a30a041..b75e7f137d8 100644 --- a/apps/dav/lib/connector/sabre/fakelockerplugin.php +++ b/apps/dav/lib/connector/sabre/fakelockerplugin.php @@ -22,9 +22,9 @@ namespace OCA\DAV\Connector\Sabre; use Sabre\DAV\Locks\LockInfo; -use Sabre\DAV\Property\LockDiscovery; -use Sabre\DAV\Property\SupportedLock; use Sabre\DAV\ServerPlugin; +use Sabre\DAV\Xml\Property\LockDiscovery; +use Sabre\DAV\Xml\Property\SupportedLock; use Sabre\HTTP\RequestInterface; use Sabre\HTTP\ResponseInterface; use Sabre\DAV\PropFind; @@ -122,12 +122,6 @@ class FakeLockerPlugin extends ServerPlugin { */ public function fakeLockProvider(RequestInterface $request, ResponseInterface $response) { - $dom = new \DOMDocument('1.0', 'utf-8'); - $prop = $dom->createElementNS('DAV:', 'd:prop'); - $dom->appendChild($prop); - - $lockDiscovery = $dom->createElementNS('DAV:', 'd:lockdiscovery'); - $prop->appendChild($lockDiscovery); $lockInfo = new LockInfo(); $lockInfo->token = md5($request->getPath()); @@ -135,10 +129,12 @@ class FakeLockerPlugin extends ServerPlugin { $lockInfo->depth = \Sabre\DAV\Server::DEPTH_INFINITY; $lockInfo->timeout = 1800; - $lockObj = new LockDiscovery([$lockInfo]); - $lockObj->serialize($this->server, $lockDiscovery); + $body = $this->server->xml->write('{DAV:}prop', [ + '{DAV:}lockdiscovery' => + new LockDiscovery([$lockInfo]) + ]); - $response->setBody($dom->saveXML()); + $response->setBody($body); return false; } diff --git a/apps/dav/lib/connector/sabre/taglist.php b/apps/dav/lib/connector/sabre/taglist.php index 177cc23e805..1b32d4b1047 100644 --- a/apps/dav/lib/connector/sabre/taglist.php +++ b/apps/dav/lib/connector/sabre/taglist.php @@ -22,82 +22,100 @@ namespace OCA\DAV\Connector\Sabre; -use Sabre\DAV; +use Sabre\Xml\Element; +use Sabre\Xml\Reader; +use Sabre\Xml\Writer; /** * TagList property * * This property contains multiple "tag" elements, each containing a tag name. */ -class TagList extends DAV\Property { +class TagList implements Element { const NS_OWNCLOUD = 'http://owncloud.org/ns'; - /** - * tags - * - * @var array - */ - private $tags; - - /** - * @param array $tags - */ - public function __construct(array $tags) { - $this->tags = $tags; - } - - /** - * Returns the tags - * - * @return array - */ - public function getTags() { - - return $this->tags; - - } - - /** - * Serializes this property. - * - * @param DAV\Server $server - * @param \DOMElement $dom - * @return void - */ - public function serialize(DAV\Server $server,\DOMElement $dom) { - - $prefix = $server->xmlNamespaces[self::NS_OWNCLOUD]; - - foreach($this->tags as $tag) { - - $elem = $dom->ownerDocument->createElement($prefix . ':tag'); - $elem->appendChild($dom->ownerDocument->createTextNode($tag)); - - $dom->appendChild($elem); - } - - } - - /** - * Unserializes this property from a DOM Element - * - * This method returns an instance of this class. - * It will only decode tag values. - * - * @param \DOMElement $dom - * @param array $propertyMap - * @return \OCA\DAV\Connector\Sabre\TagList - */ - static function unserialize(\DOMElement $dom, array $propertyMap) { - - $tags = array(); - foreach($dom->childNodes as $child) { - if (DAV\XMLUtil::toClarkNotation($child)==='{' . self::NS_OWNCLOUD . '}tag') { - $tags[] = $child->textContent; - } - } - return new self($tags); - - } - + /** + * tags + * + * @var array + */ + private $tags; + + /** + * @param array $tags + */ + public function __construct(array $tags) { + $this->tags = $tags; + } + + /** + * Returns the tags + * + * @return array + */ + public function getTags() { + + return $this->tags; + + } + + /** + * The deserialize method is called during xml parsing. + * + * This method is called statictly, this is because in theory this method + * may be used as a type of constructor, or factory method. + * + * Often you want to return an instance of the current class, but you are + * free to return other data as well. + * + * You are responsible for advancing the reader to the next element. Not + * doing anything will result in a never-ending loop. + * + * If you just want to skip parsing for this element altogether, you can + * just call $reader->next(); + * + * $reader->parseInnerTree() will parse the entire sub-tree, and advance to + * the next element. + * + * @param Reader $reader + * @return mixed + */ + static function xmlDeserialize(Reader $reader) { + $tags = []; + + foreach ($reader->parseInnerTree() as $elem) { + if ($elem['name'] === '{' . self::NS_OWNCLOUD . '}tag') { + $tags[] = $elem['value']; + } + } + return new self($tags); + } + + /** + * The xmlSerialize metod is called during xml writing. + * + * Use the $writer argument to write its own xml serialization. + * + * An important note: do _not_ create a parent element. Any element + * implementing XmlSerializble should only ever write what's considered + * its 'inner xml'. + * + * The parent of the current element is responsible for writing a + * containing element. + * + * This allows serializers to be re-used for different element names. + * + * If you are opening new elements, you must also close them again. + * + * @param Writer $writer + * @return void + */ + function xmlSerialize(Writer $writer) { + + foreach ($this->tags as $tag) { + $writer->startElement(self::NS_OWNCLOUD . ':tag'); + $writer->writeElement($tag); + $writer->endElement(); + } + } } diff --git a/apps/dav/tests/unit/caldav/caldavbackendtest.php b/apps/dav/tests/unit/caldav/caldavbackendtest.php index 258c5627ad9..e9483a47a78 100644 --- a/apps/dav/tests/unit/caldav/caldavbackendtest.php +++ b/apps/dav/tests/unit/caldav/caldavbackendtest.php @@ -23,9 +23,9 @@ namespace Tests\Connector\Sabre; use DateTime; use DateTimeZone; use OCA\DAV\CalDAV\CalDavBackend; -use Sabre\CalDAV\Property\SupportedCalendarComponentSet; -use Sabre\DAV\Property\Href; +use Sabre\CalDAV\Xml\Property\SupportedCalendarComponentSet; use Sabre\DAV\PropPatch; +use Sabre\DAV\Xml\Property\Href; use Test\TestCase; /** diff --git a/apps/dav/tests/unit/connector/sabre/FakeLockerPluginTest.php b/apps/dav/tests/unit/connector/sabre/FakeLockerPluginTest.php index dfe8cc220a3..8539e9c06ee 100644 --- a/apps/dav/tests/unit/connector/sabre/FakeLockerPluginTest.php +++ b/apps/dav/tests/unit/connector/sabre/FakeLockerPluginTest.php @@ -21,6 +21,7 @@ namespace OCA\DAV\Tests\Unit\Connector\Sabre; use OCA\DAV\Connector\Sabre\FakeLockerPlugin; +use Sabre\HTTP\Response; use Test\TestCase; /** @@ -141,20 +142,19 @@ class FakeLockerPluginTest extends TestCase { public function testFakeLockProvider() { $request = $this->getMock('\Sabre\HTTP\RequestInterface'); - $response = $this->getMock('\Sabre\HTTP\ResponseInterface'); + $response = new Response(); $server = $this->getMock('\Sabre\DAV\Server'); $this->fakeLockerPlugin->initialize($server); $request->expects($this->exactly(2)) ->method('getPath') ->will($this->returnValue('MyPath')); - $response->expects($this->once()) - ->method('setBody') - ->with(' -MyPathinfinitySecond-1800opaquelocktoken:fe4f7f2437b151fbcb4e9f5c8118c6b1 -'); $this->assertSame(false, $this->fakeLockerPlugin->fakeLockProvider($request, $response)); + + $expectedXml = 'MyPathinfinitySecond-1800opaquelocktoken:fe4f7f2437b151fbcb4e9f5c8118c6b1'; + + $this->assertXmlStringEqualsXmlString($expectedXml, $response->getBody()); } public function testFakeUnlockProvider() { diff --git a/apps/dav/tests/unit/connector/sabre/auth.php b/apps/dav/tests/unit/connector/sabre/auth.php index 4c060ff04bb..595bd441617 100644 --- a/apps/dav/tests/unit/connector/sabre/auth.php +++ b/apps/dav/tests/unit/connector/sabre/auth.php @@ -249,9 +249,12 @@ class Auth extends TestCase { } public function testAuthenticateAlreadyLoggedIn() { - $server = $this->getMockBuilder('\Sabre\DAV\Server') - ->disableOriginalConstructor() - ->getMock(); + $request = $this->getMockBuilder('Sabre\HTTP\RequestInterface') + ->disableOriginalConstructor() + ->getMock(); + $response = $this->getMockBuilder('Sabre\HTTP\ResponseInterface') + ->disableOriginalConstructor() + ->getMock(); $this->userSession ->expects($this->once()) ->method('isLoggedIn') @@ -275,13 +278,10 @@ class Auth extends TestCase { ->expects($this->once()) ->method('close'); - $this->assertTrue($this->auth->authenticate($server, 'TestRealm')); + $response = $this->auth->check($request, $response); + $this->assertEquals([true, 'principals/MyWrongDavUser'], $response); } - /** - * @expectedException \Sabre\DAV\Exception\NotAuthenticated - * @expectedExceptionMessage No basic authentication headers were found - */ public function testAuthenticateNoBasicAuthenticateHeadersProvided() { $server = $this->getMockBuilder('\Sabre\DAV\Server') ->disableOriginalConstructor() @@ -292,7 +292,8 @@ class Auth extends TestCase { $server->httpResponse = $this->getMockBuilder('\Sabre\HTTP\ResponseInterface') ->disableOriginalConstructor() ->getMock(); - $this->auth->authenticate($server, 'TestRealm'); + $response = $this->auth->check($server->httpRequest, $server->httpResponse); + $this->assertEquals([false, 'No \'Authorization: Basic\' header found. Either the client didn\'t send one, or the server is mis-configured'], $response); } /** @@ -300,21 +301,20 @@ class Auth extends TestCase { * @expectedExceptionMessage Cannot authenticate over ajax calls */ public function testAuthenticateNoBasicAuthenticateHeadersProvidedWithAjax() { - $server = $this->getMockBuilder('\Sabre\DAV\Server') + /** @var \Sabre\HTTP\RequestInterface $httpRequest */ + $httpRequest = $this->getMockBuilder('\Sabre\HTTP\RequestInterface') ->disableOriginalConstructor() ->getMock(); - $server->httpRequest = $this->getMockBuilder('\Sabre\HTTP\RequestInterface') + /** @var \Sabre\HTTP\ResponseInterface $httpResponse */ + $httpResponse = $this->getMockBuilder('\Sabre\HTTP\ResponseInterface') ->disableOriginalConstructor() ->getMock(); - $server->httpResponse = $this->getMockBuilder('\Sabre\HTTP\ResponseInterface') - ->disableOriginalConstructor() - ->getMock(); - $server->httpRequest + $httpRequest ->expects($this->once()) ->method('getHeader') ->with('X-Requested-With') ->will($this->returnValue('XMLHttpRequest')); - $this->auth->authenticate($server, 'TestRealm'); + $this->auth->check($httpRequest, $httpResponse); } public function testAuthenticateValidCredentials() { @@ -352,13 +352,10 @@ class Auth extends TestCase { ->expects($this->exactly(2)) ->method('getUser') ->will($this->returnValue($user)); - $this->assertTrue($this->auth->authenticate($server, 'TestRealm')); + $response = $this->auth->check($server->httpRequest, $server->httpResponse); + $this->assertEquals([true, 'principals/username'], $response); } - /** - * @expectedException \Sabre\DAV\Exception\NotAuthenticated - * @expectedExceptionMessage Username or password does not match - */ public function testAuthenticateInvalidCredentials() { $server = $this->getMockBuilder('\Sabre\DAV\Server') ->disableOriginalConstructor() @@ -384,6 +381,7 @@ class Auth extends TestCase { ->method('login') ->with('username', 'password') ->will($this->returnValue(false)); - $this->auth->authenticate($server, 'TestRealm'); + $response = $this->auth->check($server->httpRequest, $server->httpResponse); + $this->assertEquals([false, 'Username or password was incorrect'], $response); } } diff --git a/apps/dav/tests/unit/connector/sabre/requesttest/auth.php b/apps/dav/tests/unit/connector/sabre/requesttest/auth.php index 02b64ab070b..3caa019af8d 100644 --- a/apps/dav/tests/unit/connector/sabre/requesttest/auth.php +++ b/apps/dav/tests/unit/connector/sabre/requesttest/auth.php @@ -9,6 +9,8 @@ namespace OCA\DAV\Tests\Unit\Connector\Sabre\RequestTest; use Sabre\DAV\Auth\Backend\BackendInterface; +use Sabre\HTTP\RequestInterface; +use Sabre\HTTP\ResponseInterface; class Auth implements BackendInterface { /** @@ -32,18 +34,35 @@ class Auth implements BackendInterface { $this->password = $password; } - /** - * Authenticates the user based on the current request. + * When this method is called, the backend must check if authentication was + * successful. + * + * The returned value must be one of the following + * + * [true, "principals/username"] + * [false, "reason for failure"] + * + * If authentication was successful, it's expected that the authentication + * backend returns a so-called principal url. + * + * Examples of a principal url: * - * If authentication is successful, true must be returned. - * If authentication fails, an exception must be thrown. + * principals/admin + * principals/user1 + * principals/users/joe + * principals/uid/123457 * - * @param \Sabre\DAV\Server $server - * @param string $realm - * @return boolean|null + * If you don't use WebDAV ACL (RFC3744) we recommend that you simply + * return a string such as: + * + * principals/users/[username] + * + * @param RequestInterface $request + * @param ResponseInterface $response + * @return array */ - function authenticate(\Sabre\DAV\Server $server, $realm) { + function check(RequestInterface $request, ResponseInterface $response) { $userSession = \OC::$server->getUserSession(); $result = $userSession->login($this->user, $this->password); if ($result) { @@ -52,18 +71,33 @@ class Auth implements BackendInterface { \OC_Util::setupFS($user); //trigger creation of user home and /files folder \OC::$server->getUserFolder($user); + return [true, "principals/$user"]; } - return $result; + return [false, "login failed"]; } /** - * Returns information about the currently logged in username. + * This method is called when a user could not be authenticated, and + * authentication was required for the current request. + * + * This gives you the opportunity to set authentication headers. The 401 + * status code will already be set. + * + * In this case of Basic Auth, this would for example mean that the + * following header needs to be set: + * + * $response->addHeader('WWW-Authenticate', 'Basic realm=SabreDAV'); * - * If nobody is currently logged in, this method should return null. + * Keep in mind that in the case of multiple authentication backends, other + * WWW-Authenticate headers may already have been set, and you'll want to + * append your own WWW-Authenticate header instead of overwriting the + * existing one. * - * @return string + * @param RequestInterface $request + * @param ResponseInterface $response + * @return void */ - function getCurrentUser() { - return $this->user; + function challenge(RequestInterface $request, ResponseInterface $response) { + // TODO: Implement challenge() method. } } diff --git a/lib/private/files/storage/dav.php b/lib/private/files/storage/dav.php index dcde7b8029b..9147f572461 100644 --- a/lib/private/files/storage/dav.php +++ b/lib/private/files/storage/dav.php @@ -47,6 +47,7 @@ use OCP\Files\StorageNotAvailableException; use OCP\Util; use Sabre\DAV\Client; use Sabre\DAV\Exception\NotFound; +use Sabre\DAV\Xml\Property\ResourceType; use Sabre\HTTP\ClientException; use Sabre\HTTP\ClientHttpException; @@ -137,7 +138,7 @@ class DAV extends Common { $this->client->setThrowExceptions(true); if ($this->secure === true && $this->certPath) { - $this->client->addTrustedCertificates($this->certPath); + $this->client->addCurlSetting(CURLOPT_CAINFO, $this->certPath); } } @@ -280,7 +281,8 @@ class DAV extends Common { $response = $this->propfind($path); $responseType = array(); if (isset($response["{DAV:}resourcetype"])) { - $responseType = $response["{DAV:}resourcetype"]->resourceType; + /** @var ResourceType[] $response */ + $responseType = $response["{DAV:}resourcetype"]->getValue(); } return (count($responseType) > 0 and $responseType[0] == "{DAV:}collection") ? 'dir' : 'file'; } catch (ClientHttpException $e) { @@ -554,7 +556,8 @@ class DAV extends Common { $response = $this->propfind($path); $responseType = array(); if (isset($response["{DAV:}resourcetype"])) { - $responseType = $response["{DAV:}resourcetype"]->resourceType; + /** @var ResourceType[] $response */ + $responseType = $response["{DAV:}resourcetype"]->getValue(); } $type = (count($responseType) > 0 and $responseType[0] == "{DAV:}collection") ? 'dir' : 'file'; if ($type == 'dir') { -- 2.39.5