From 5dfd8f6549dbacc63009d1c372255ca5ce7d965b Mon Sep 17 00:00:00 2001 From: Daniel Kesselberg Date: Mon, 12 Jun 2023 11:26:23 +0200 Subject: [PATCH 1/2] perf: skip request without write permission Signed-off-by: Daniel Kesselberg --- apps/dav/lib/Connector/Sabre/DavAclPlugin.php | 15 ++++- .../features/bootstrap/CalDavContext.php | 25 ++++++++ .../features/bootstrap/CardDavContext.php | 61 +++++++++++++++++++ build/integration/features/caldav.feature | 18 +++++- build/integration/features/carddav.feature | 15 +++++ 5 files changed, 131 insertions(+), 3 deletions(-) diff --git a/apps/dav/lib/Connector/Sabre/DavAclPlugin.php b/apps/dav/lib/Connector/Sabre/DavAclPlugin.php index 6842975835dfd..7fa94d7b90398 100644 --- a/apps/dav/lib/Connector/Sabre/DavAclPlugin.php +++ b/apps/dav/lib/Connector/Sabre/DavAclPlugin.php @@ -94,8 +94,19 @@ public function beforeMethod(RequestInterface $request, ResponseInterface $respo $path = $request->getPath(); // prevent the plugin from causing an unneeded overhead for file requests - if (strpos($path, 'files/') !== 0) { - parent::beforeMethod($request, $response); + if (str_starts_with($path, 'files/')) { + return; + } + + parent::beforeMethod($request, $response); + + $createAddressbookOrCalendarRequest = ($request->getMethod() === 'MKCALENDAR' || $request->getMethod() === 'MKCOL') + && (str_starts_with($path, 'addressbooks/') || str_starts_with($path, 'calendars/')); + + if ($createAddressbookOrCalendarRequest) { + [$parentName] = \Sabre\Uri\split($path); + // is calendars/users/bob or addressbooks/users/bob writeable? + $this->checkPrivileges($parentName, '{DAV:}write'); } } } diff --git a/build/integration/features/bootstrap/CalDavContext.php b/build/integration/features/bootstrap/CalDavContext.php index 49d8c8e596357..936463b579ef4 100644 --- a/build/integration/features/bootstrap/CalDavContext.php +++ b/build/integration/features/bootstrap/CalDavContext.php @@ -27,6 +27,7 @@ require __DIR__ . '/../../vendor/autoload.php'; use GuzzleHttp\Client; +use GuzzleHttp\Exception\GuzzleException; use Psr\Http\Message\ResponseInterface; class CalDavContext implements \Behat\Behat\Context\Context { @@ -233,4 +234,28 @@ public function t($amount) { ); } } + + /** + * @When :user sends a create calendar request to :calendar on the endpoint :endpoint + */ + public function sendsCreateCalendarRequest(string $user, string $calendar, string $endpoint) { + $davUrl = $this->baseUrl . $endpoint . $calendar; + $password = ($user === 'admin') ? 'admin' : '123456'; + + try { + $this->response = $this->client->request( + 'MKCALENDAR', + $davUrl, + [ + 'body' => 'test1#21213D', + 'auth' => [ + $user, + $password, + ], + ] + ); + } catch (GuzzleException $e) { + $this->response = $e->getResponse(); + } + } } diff --git a/build/integration/features/bootstrap/CardDavContext.php b/build/integration/features/bootstrap/CardDavContext.php index 18a9f3dd24958..80d96215ebaae 100644 --- a/build/integration/features/bootstrap/CardDavContext.php +++ b/build/integration/features/bootstrap/CardDavContext.php @@ -26,6 +26,7 @@ require __DIR__ . '/../../vendor/autoload.php'; use GuzzleHttp\Client; +use GuzzleHttp\Exception\GuzzleException; use GuzzleHttp\Message\ResponseInterface; class CardDavContext implements \Behat\Behat\Context\Context { @@ -311,4 +312,64 @@ public function theFollowingHttpHeadersShouldBeSet(\Behat\Gherkin\Node\TableNode } } } + + /** + * @When :user sends a create addressbook request to :addressbook on the endpoint :endpoint + */ + public function sendsCreateAddressbookRequest(string $user, string $addressbook, string $endpoint) { + $davUrl = $this->baseUrl . $endpoint . $addressbook; + $password = ($user === 'admin') ? 'admin' : '123456'; + + try { + $this->response = $this->client->request( + 'MKCOL', + $davUrl, + [ + 'body' => ' + + + + , + ,' . $addressbook . ' + + + ', + 'auth' => [ + $user, + $password, + ], + 'headers' => [ + 'Content-Type' => 'application/xml;charset=UTF-8', + ], + ] + ); + } catch (GuzzleException $e) { + $this->response = $e->getResponse(); + } + } + + /** + * @Then The CardDAV HTTP status code should be :code + * @param int $code + * @throws \Exception + */ + public function theCarddavHttpStatusCodeShouldBe($code) { + if ((int)$code !== $this->response->getStatusCode()) { + throw new \Exception( + sprintf( + 'Expected %s got %s', + (int)$code, + $this->response->getStatusCode() + ) + ); + } + + $body = $this->response->getBody()->getContents(); + if ($body && substr($body, 0, 1) === '<') { + $reader = new Sabre\Xml\Reader(); + $reader->xml($body); + $this->responseXml = $reader->parse(); + } + } } diff --git a/build/integration/features/caldav.feature b/build/integration/features/caldav.feature index 2bddbc3e9e408..e2cb4f8dc9235 100644 --- a/build/integration/features/caldav.feature +++ b/build/integration/features/caldav.feature @@ -58,4 +58,20 @@ Feature: caldav Then The CalDAV HTTP status code should be "202" When "admin" requests calendar "/" on the endpoint "/remote.php/dav/public-calendars" Then The CalDAV HTTP status code should be "207" - Then There should be "0" calendars in the response body \ No newline at end of file + Then There should be "0" calendars in the response body + + Scenario: Create calendar request for non-existing calendar of another user + Given user "user0" exists + When "user0" sends a create calendar request to "admin/MyCalendar2" on the endpoint "/remote.php/dav/calendars/" + Then The CalDAV HTTP status code should be "404" + And The exception is "Sabre\DAV\Exception\NotFound" + And The error message is "Node with name 'admin' could not be found" + + Scenario: Create calendar request for existing calendar of another user + Given user "user0" exists + When "admin" creates a calendar named "MyCalendar2" + Then The CalDAV HTTP status code should be "201" + When "user0" sends a create calendar request to "admin/MyCalendar2" on the endpoint "/remote.php/dav/calendars/" + Then The CalDAV HTTP status code should be "404" + And The exception is "Sabre\DAV\Exception\NotFound" + And The error message is "Node with name 'admin' could not be found" diff --git a/build/integration/features/carddav.feature b/build/integration/features/carddav.feature index e0c11ec8dc19a..9c9df6ddd94be 100644 --- a/build/integration/features/carddav.feature +++ b/build/integration/features/carddav.feature @@ -62,3 +62,18 @@ Feature: carddav |X-Permitted-Cross-Domain-Policies|none| |X-Robots-Tag|noindex, nofollow| |X-XSS-Protection|1; mode=block| + + Scenario: Create addressbook request for non-existing addressbook of another user + Given user "user0" exists + When "user0" sends a create addressbook request to "admin/MyAddressbook2" on the endpoint "/remote.php/dav/addressbooks/" + Then The CardDAV HTTP status code should be "404" + And The CardDAV exception is "Sabre\DAV\Exception\NotFound" + And The CardDAV error message is "File not found: admin in 'addressbooks'" + + Scenario: Create addressbook request for existing addressbook of another user + Given user "user0" exists + When "admin" creates an addressbook named "MyAddressbook2" with statuscode "201" + When "user0" sends a create addressbook request to "admin/MyAddressbook2" on the endpoint "/remote.php/dav/addressbooks/" + Then The CardDAV HTTP status code should be "404" + And The CardDAV exception is "Sabre\DAV\Exception\NotFound" + And The CardDAV error message is "File not found: admin in 'addressbooks'" From f19fdc5325488f3515d168fc169f84b2e8e8159c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julius=20H=C3=A4rtl?= Date: Thu, 2 Mar 2023 23:23:57 +0100 Subject: [PATCH 2/2] chore(psalm): Make psalm aware of sabre/uri MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Julius Härtl --- psalm.xml | 1 + 1 file changed, 1 insertion(+) diff --git a/psalm.xml b/psalm.xml index dac8635b5be4a..009c59c785747 100644 --- a/psalm.xml +++ b/psalm.xml @@ -77,6 +77,7 @@ +