chore

Author: tchapi

.env

@@ -85,6 +85,10 @@ INVITE_FROM_ADDRESS=no-reply@example.org
 # USE ABSOLUTE PATHS for better predictability
 WEBDAV_TMP_DIR='/webdav/tmp'
 WEBDAV_PUBLIC_DIR='/webdav/public'
+# The public directory is readable by every authenticated user. By default only admins
+# (users flagged as such in the dashboard) can create, modify or delete files in it.
+# Set this to true to let every authenticated user write to it (shared drop folder).
+WEBDAV_PUBLIC_DIR_WRITABLE=false
 # By default, home directories are disabled totally (env var set to an empty string).
 # If needed, it is recommended to use a folder that is NOT a child of the public dir,
 # such as /webdav/homes for instance, so that users cannot access other users' homes.

README.md

@@ -194,9 +194,14 @@ BIRTHDAY_REMINDER_OFFSET=false
 ```shell
 WEBDAV_TMP_DIR=/webdav/tmp
 WEBDAV_PUBLIC_DIR=/webdav/public
+WEBDAV_PUBLIC_DIR_WRITABLE=false
 WEBDAV_HOMES_DIR=
 ```
 
+> [!NOTE]
+>
+> The public directory (served at `/dav/public`) is readable by every authenticated user. By default only users flagged as admins in the dashboard can create, modify or delete files in it; set `WEBDAV_PUBLIC_DIR_WRITABLE=true` to let every authenticated user write to it. Directories must be absolute paths and must not live inside the web root.
+
 > [!NOTE]
 >
 > In a docker setup, I recommend setting `WEBDAV_TMP_DIR` to `/tmp`.

config/services.yaml

@@ -12,6 +12,7 @@ parameters:
     birthday_reminder_offset: '%env(default:default_birthday_reminder_offset:BIRTHDAY_REMINDER_OFFSET)%'
     default_birthday_reminder_offset: "PT9H"
     caldav_enabled: "%env(bool:CALDAV_ENABLED)%"
+    default_webdav_public_dir_writable: "false"
     carddav_enabled: "%env(bool:CARDDAV_ENABLED)%"
 
 services:
@@ -64,6 +65,7 @@ services:
             $webdavPublicDir: "%env(resolve:WEBDAV_PUBLIC_DIR)%"
             $webdavHomesDir: "%env(resolve:WEBDAV_HOMES_DIR)%"
             $webdavTmpDir: "%env(resolve:WEBDAV_TMP_DIR)%"
+            $webdavPublicDirWritable: "%env(bool:default:default_webdav_public_dir_writable:WEBDAV_PUBLIC_DIR_WRITABLE)%"
 
     App\Security\LoginFormAuthenticator:
         arguments:

docker/.env

@@ -47,6 +47,7 @@ LDAP_CERTIFICATE_CHECKING_STRATEGY=try # never, hard, demand, try, or allow
 # WebDAV settings
 WEBDAV_TMP_DIR=/webdav/tmp
 WEBDAV_PUBLIC_DIR=/webdav/public
+WEBDAV_PUBLIC_DIR_WRITABLE=false
 WEBDAV_HOMES_DIR=
 
 # Mail settings

src/Controller/DAVController.php

@@ -6,6 +6,7 @@
 use App\Entity\User;
 use App\Plugins\BirthdayCalendarPlugin;
 use App\Plugins\DavisIMipPlugin;
+use App\Plugins\DavisTemporaryFileFilterPlugin;
 use App\Plugins\PublicAwareDAVACLPlugin;
 use App\Services\BasicAuth;
 use App\Services\BirthdayService;
@@ -92,6 +93,14 @@ class DAVController extends AbstractController
      */
     protected $webdavTmpDir;
 
+    /**
+     * Can every authenticated user write to the WebDAV public directory
+     * (otherwise only admins can, everybody can read).
+     *
+     * @var bool
+     */
+    protected $webdavPublicDirWritable;
+
     /**
      * @var EntityManagerInterface
      */
@@ -149,7 +158,7 @@ class DAVController extends AbstractController
      */
     protected $server;
 
-    public function __construct(MailerInterface $mailer, BasicAuth $basicAuthBackend, IMAPAuth $IMAPAuthBackend, LDAPAuth $LDAPAuthBackend, UrlGeneratorInterface $router, EntityManagerInterface $entityManager, LoggerInterface $logger, BirthdayService $birthdayService, string $publicDir, bool $calDAVEnabled = true, bool $cardDAVEnabled = true, bool $webDAVEnabled = false, bool $publicCalendarsEnabled = true, ?string $inviteAddress = null, ?string $authMethod = null, ?string $authRealm = null, ?string $webdavPublicDir = null, ?string $webdavHomesDir = null, ?string $webdavTmpDir = null)
+    public function __construct(MailerInterface $mailer, BasicAuth $basicAuthBackend, IMAPAuth $IMAPAuthBackend, LDAPAuth $LDAPAuthBackend, UrlGeneratorInterface $router, EntityManagerInterface $entityManager, LoggerInterface $logger, BirthdayService $birthdayService, string $publicDir, bool $calDAVEnabled = true, bool $cardDAVEnabled = true, bool $webDAVEnabled = false, bool $publicCalendarsEnabled = true, ?string $inviteAddress = null, ?string $authMethod = null, ?string $authRealm = null, ?string $webdavPublicDir = null, ?string $webdavHomesDir = null, ?string $webdavTmpDir = null, bool $webdavPublicDirWritable = false)
     {
         $this->publicDir = $publicDir;
 
@@ -162,6 +171,7 @@ public function __construct(MailerInterface $mailer, BasicAuth $basicAuthBackend
         $this->webdavPublicDir = $webdavPublicDir;
         $this->webdavHomesDir = $webdavHomesDir;
         $this->webdavTmpDir = $webdavTmpDir;
+        $this->webdavPublicDirWritable = $webdavPublicDirWritable;
 
         $this->em = $entityManager;
         $this->logger = $logger;
@@ -222,6 +232,7 @@ private function initServer(string $authMethod, string $authRealm = User::DEFAUL
         ];
 
         if ($this->webdavHomesDir) {
+            $this->assertWebdavDirectory($this->webdavHomesDir, 'WEBDAV_HOMES_DIR');
             $nodes[] = new \Sabre\DAVACL\FS\HomeCollection($principalBackend, $this->webdavHomesDir);
         }
 
@@ -234,7 +245,19 @@ private function initServer(string $authMethod, string $authRealm = User::DEFAUL
             $nodes[] = new \Sabre\CardDAV\AddressBookRoot($principalBackend, $carddavBackend);
         }
         if ($this->webDAVEnabled && $this->webdavTmpDir && $this->webdavPublicDir) {
-            $nodes[] = new \Sabre\DAV\FS\Directory($this->webdavPublicDir);
+            $this->assertWebdavDirectory($this->webdavTmpDir, 'WEBDAV_TMP_DIR');
+            $this->assertWebdavDirectory($this->webdavPublicDir, 'WEBDAV_PUBLIC_DIR');
+
+            // Explicit ACL for the shared directory: every authenticated user can read it, and
+            // writing is reserved to admins (the ACL plugin grants them every privilege) unless
+            // WEBDAV_PUBLIC_DIR_WRITABLE opens it to everyone. Children inherit this ACL.
+            $publicDirAcl = [
+                ['principal' => '{DAV:}authenticated', 'privilege' => '{DAV:}read', 'protected' => true],
+            ];
+            if ($this->webdavPublicDirWritable) {
+                $publicDirAcl[] = ['principal' => '{DAV:}authenticated', 'privilege' => '{DAV:}write', 'protected' => true];
+            }
+            $nodes[] = new \Sabre\DAVACL\FS\Collection($this->webdavPublicDir, $publicDirAcl);
         }
 
         // The object tree needs in turn to be passed to the server class
@@ -287,13 +310,34 @@ private function initServer(string $authMethod, string $authRealm = User::DEFAUL
 
         // WebDAV plugins
         if ($this->webDAVEnabled && $this->webdavTmpDir && $this->webdavPublicDir) {
-            if (!is_dir($this->webdavTmpDir) || !is_dir($this->webdavPublicDir)) {
-                throw new \Exception('The WebDAV temp dir and/or public dir are not available. Make sure they are created with the correct permissions.');
-            }
             $lockBackend = new \Sabre\DAV\Locks\Backend\File($this->webdavTmpDir.'/locksdb');
             $this->server->addPlugin(new \Sabre\DAV\Locks\Plugin($lockBackend));
             $this->server->addPlugin(new \Sabre\DAV\Browser\GuessContentType());
-            $this->server->addPlugin(new \Sabre\DAV\TemporaryFileFilterPlugin($this->webdavTmpDir));
+            // Temporary files must obey the ACL of their directory (see the plugin for the why)
+            $this->server->addPlugin(new DavisTemporaryFileFilterPlugin($this->webdavTmpDir));
+        }
+    }
+
+    /**
+     * A WebDAV directory must exist, be given as an absolute path (a relative one would be
+     * resolved against the PHP process' working directory, which is not predictable) and
+     * must not live inside the web root, where the web server would serve its content
+     * directly and bypass every DAV permission check.
+     */
+    private function assertWebdavDirectory(string $dir, string $envVar): void
+    {
+        if (!str_starts_with($dir, '/')) {
+            throw new \RuntimeException(sprintf('%s must be an absolute path, "%s" given.', $envVar, $dir));
+        }
+
+        $realDir = realpath($dir);
+        if (false === $realDir || !is_dir($realDir)) {
+            throw new \RuntimeException(sprintf('%s points to "%s", which does not exist or is not a directory. Make sure it is created with the correct permissions.', $envVar, $dir));
+        }
+
+        $webRoot = realpath($this->publicDir);
+        if (false !== $webRoot && ($realDir === $webRoot || str_starts_with($realDir.'/', $webRoot.'/'))) {
+            throw new \RuntimeException(sprintf('%s ("%s") must not be inside the web root ("%s"): the web server would serve these files without any permission check.', $envVar, $dir, $webRoot));
         }
     }
 
@@ -365,38 +409,38 @@ public function dav(Request $request, ?string $path, ?Profiler $profiler = null)
             return $response;
         }
 
-        // \Sabre\DAV\Server does not let us use a custom SAPI, and its behaviour
-        // is to directly output headers and content to php://output. Hence, we
-        // let the headers pass (we have not choice) and capture the output in a
-        // buffer.
-        // This allows us to use a Response, and not to break the events triggered
-        // by Symfony after the response is sent, like for instance the TERMINATE
-        // event from the Kernel, that is used to send emails...
-
+        // \Sabre\DAV\Server does not let us use a custom SAPI: it writes its status line and
+        // headers with header() and streams the body to php://output. We capture the output
+        // so that we can hand a proper Response back to Symfony (and keep its kernel events,
+        // like TERMINATE, working).
         ob_start(); // Does not capture headers!
         $this->server->start();
-
-        $output = ob_get_contents();
-        ob_end_clean();
-
-        // As previously said, headers are already _prepared_ by the server,
-        // so we can't modify them or remove them. But they are not _sent_ yet,
-        // so headers_sent() is false, and Symfony will add its own headers above it.
-        //
-        // The Content-type header is the problem, since Symfony will
-        // output `text/html` for everything since it doesn't know any better.
-        // Thus, we have to get the _real_ Content-type header already prepared,
-        // and force it in the Symfony Response.
-        //
-        // That's what we do here.
-        $response = new Response($output, http_response_code(), []);
-        foreach (headers_list() as $header) {
-            if ('content-type:' === strtolower(substr($header, 0, 13))) {
-                $headerArray = explode(':', $header);
-                $response->headers->set('Content-type', $headerArray[1]);
+        $output = ob_get_clean();
+
+        // Some plugins short-circuit a request by returning false from `beforeMethod` (the
+        // temporary file filter does, for .DS_Store and friends). sabre then never sends
+        // anything: status, headers and body only exist in its response object. So we always
+        // rebuild the Symfony response from that object, falling back to its body when
+        // nothing was streamed.
+        $sabreResponse = $this->server->httpResponse;
+        if ('' === $output) {
+            $body = $sabreResponse->getBody();
+            // A stream that sabre already sent has been closed (is_resource() is then false):
+            // only read bodies that were never streamed.
+            if (is_string($body) || (is_resource($body) && 'stream' === get_resource_type($body))) {
+                $output = $sabreResponse->getBodyAsString();
             }
         }
 
+        // Drop the headers sabre may already have queued with header(): Symfony re-sends the
+        // very same ones from the Response below, and would otherwise duplicate them.
+        header_remove();
+
+        $response = new Response($output, $sabreResponse->getStatus());
+        foreach ($sabreResponse->getHeaders() as $name => $values) {
+            $response->headers->set($name, $values);
+        }
+
         return $response;
     }
 }

src/Plugins/DavisTemporaryFileFilterPlugin.php

@@ -0,0 +1,46 @@
+<?php
+
+namespace App\Plugins;
+
+use Sabre\DAV\TemporaryFileFilterPlugin;
+use Sabre\DAVACL\Plugin as AclPlugin;
+use Sabre\HTTP\RequestInterface;
+use Sabre\HTTP\ResponseInterface;
+use Sabre\Uri;
+
+/**
+ * sabre/dav's TemporaryFileFilterPlugin intercepts the junk files desktop clients
+ * write next to real files (.DS_Store, Thumbs.db, ._*, *.swp, ...) and stores them
+ * outside the DAV tree. Because those files never exist in the tree, the ACL plugin
+ * never checks anything for them: without this subclass, anybody (authenticated or
+ * not) could store, read and delete such files under any path.
+ *
+ * We make temporary files obey the privileges of the directory they would live in,
+ * exactly like a real file would.
+ */
+final class DavisTemporaryFileFilterPlugin extends TemporaryFileFilterPlugin
+{
+    public function beforeMethod(RequestInterface $request, ResponseInterface $response)
+    {
+        $path = $request->getPath();
+        if (false === $this->isTempFile($path)) {
+            return;
+        }
+
+        $acl = $this->server->getPlugin('acl');
+        if ($acl instanceof AclPlugin) {
+            [$parent] = Uri\split($path);
+
+            $privilege = match ($request->getMethod()) {
+                'PUT' => '{DAV:}bind',
+                'DELETE' => '{DAV:}unbind',
+                default => '{DAV:}read',
+            };
+
+            // Throws NotAuthenticated (401) for anonymous users and NeedPrivileges (403) otherwise
+            $acl->checkPrivileges($parent ?? '', $privilege);
+        }
+
+        return parent::beforeMethod($request, $response);
+    }
+}