Fix workaround for Digest-Auth with Sabre for pristine AddressbookCol…

…lection objects (Fixes #27)
mstilkerich · Mar 23, 2024 · e18914f · e18914f
1 parent c6f7958
commit e18914f
Show file tree

Hide file tree

Showing 2 changed files with 37 additions and 30 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,5 +1,10 @@
 # Changelog for CardDAV client library for PHP ("PHP-CardDavClient")
 
+## Version 1.4.1 (to 1.4.0)
+
+- Report requests to Sabre/DAV servers with Http-Digest authentication failed if issued from an
+  AddressbookCollection object that was not use for any other (non REPORT) requests before (Fixes #27).
+
 ## Version 1.4.0 (to 1.3.0)
 
 - Support servers with multiple addressbook home locations for one principal in the Discovery service.

diff --git a/src/HttpClientAdapterGuzzle.php b/src/HttpClientAdapterGuzzle.php
@@ -76,6 +76,7 @@ class HttpClientAdapterGuzzle extends HttpClientAdapter
     /**
      * The HTTP authentication scheme to use. Null if not determined yet.
      * @var ?string
+     * @psalm-var ?lowercase-string
      */
     private $authScheme;
 
@@ -161,46 +162,47 @@ public function __construct(string $base_uri, array $httpOptions)
     public function sendRequest(string $method, string $uri, array $options = []): Psr7Response
     {
         $doAuth = $this->checkSameDomainAsBase($uri);
-        $guzzleOptions = $this->prepareGuzzleOptions($options, $doAuth);
+        $triedSabreWorkaround = false;
 
         try {
-            $response = $this->client->request($method, $uri, $guzzleOptions);
-
-            // Workaround for Sabre/DAV vs. Curl incompatibility
-            // (1) Sometimes, a REPORT is directly rejected without authentication attempt
-            if ($doAuth && $this->checkSabreCurlIncompatibility($method, $response)) {
+            /*
+             * Try the request (until first success, or error other than 401 encountered, or all auth schemes tried)
+             *   - Only once in case no authentication shall be attempted
+             *   - Retry with CURLANY in case we encounter an error that looks like Sabre/Curl incompatibility
+             *   - If we get 401 reply, try all auth schemes the server offers and we can support, until success or all
+             *     failed
+             */
+            do {
                 $guzzleOptions = $this->prepareGuzzleOptions($options, $doAuth);
                 $response = $this->client->request($method, $uri, $guzzleOptions);
-            }
-
-            if ($doAuth && $response->getStatusCode() == 401) {
-                foreach ($this->getSupportedAuthSchemes($response) as $scheme) {
-                    $this->authScheme = $scheme;
 
-                    Config::$logger->debug("Trying auth scheme $scheme");
+                if ($doAuth) {
+                    if ((!$triedSabreWorkaround) && $this->checkSabreCurlIncompatibility($method, $response)) {
+                        // try workaround
+                        $triedSabreWorkaround = true;
+                        continue;
+                    } elseif ($response->getStatusCode() == 401) {
+                        if ($this->authScheme != null) {
+                            $this->failedAuthSchemes[] = $this->authScheme;
+                        }
 
-                    $guzzleOptions = $this->prepareGuzzleOptions($options, $doAuth);
-                    $response = $this->client->request($method, $uri, $guzzleOptions);
+                        // try next auth scheme
+                        $authSchemes = $this->getSupportedAuthSchemes($response);
 
-                    if ($response->getStatusCode() != 401) {
-                        // (2) Othertimes, a REPORT without authentication is first quitted with a 401, but the
-                        //     subsequent digest authentication attempt then gets the empty-body 500 reply
-                        if ($this->checkSabreCurlIncompatibility($method, $response)) {
-                            $guzzleOptions = $this->prepareGuzzleOptions($options, $doAuth);
-                            $response = $this->client->request($method, $uri, $guzzleOptions);
+                        if (empty($authSchemes)) {
+                            Config::$logger->debug("None of the available auth schemes worked");
+                            $this->authScheme = null;
+                            break;
+                        } else {
+                            $this->authScheme = $authSchemes[0];
+                            Config::$logger->debug("Trying auth scheme " . $this->authScheme);
                         }
-
-                        break;
                     } else {
-                        $this->failedAuthSchemes[] = $scheme;
+                        break;
                     }
                 }
+            } while ($doAuth);
 
-                if ($response->getStatusCode() >= 400) {
-                    Config::$logger->debug("None of the available auth schemes worked");
-                    $this->authScheme = null;
-                }
-            }
 
             return $response;
         } catch (\GuzzleHttp\Exception\RequestException $e) {
@@ -247,8 +249,8 @@ public function sendRequest(string $method, string $uri, array $options = []): P
      * We detect the situation by the following indicators:
      *  - We have the curl extension loaded
      *  - REPORT request was sent
+     *  - DIGEST was used as auth scheme
      *  - Result status code is 500
-     *  - The server is a sabre/dav server (X-Sabre-Version header is set)
      *  - The response includes the known error message:
      *    ```xml
      *    <?xml version="1.0" encoding="utf-8"?>
@@ -265,7 +267,7 @@ private function checkSabreCurlIncompatibility(string $method, Psr7Response $res
             extension_loaded("curl")
             && $response->getStatusCode() == 500
             && strcasecmp($method, "REPORT") == 0
-            && $response->hasHeader("X-Sabre-Version")
+            && $this->authScheme == "digest"
         ) {
             $body = (string) $response->getBody();
             if (strpos($body, "The input element to parse is empty. Do not attempt to parse") !== false) {