Merge pull request #38747 from nextcloud/perf/early-exit-mkcol

perf: skip request without write permission
nextcloud · Jun 22, 2023 · e9d8dbf · e9d8dbf
2 parents 44850c2 + debd03f
commit e9d8dbf
Show file tree

Hide file tree

Showing 5 changed files with 131 additions and 3 deletions.
diff --git 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
@@ -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' => '<c:mkcalendar xmlns:c="urn:ietf:params:xml:ns:caldav" xmlns:d="DAV:" xmlns:a="http://apple.com/ns/ical/" xmlns:o="http://owncloud.org/ns"><d:set><d:prop><d:displayname>test</d:displayname><o:calendar-enabled>1</o:calendar-enabled><a:calendar-color>#21213D</a:calendar-color><c:supported-calendar-component-set><c:comp name="VEVENT"/></c:supported-calendar-component-set></d:prop></d:set></c:mkcalendar>',
+					'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
@@ -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' => '<d:mkcol xmlns:card="urn:ietf:params:xml:ns:carddav"
+				  xmlns:d="DAV:">
+		<d:set>
+		  <d:prop>
+			<d:resourcetype>
+				<d:collection />,<card:addressbook />
+			  </d:resourcetype>,<d:displayname>' . $addressbook . '</d:displayname>
+		  </d:prop>
+		</d:set>
+	  </d:mkcol>',
+					'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
@@ -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
+    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
@@ -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'"