Skip to content

Commit cb29fd2

Browse files
tchapitchapi
andauthored
Fixes (#289)
Co-authored-by: tchapi <regbasket@gmail.com>
1 parent b9dd88c commit cb29fd2

21 files changed

Lines changed: 543 additions & 67 deletions

File tree

.github/workflows/ci.yml

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -112,13 +112,15 @@ jobs:
112112
- name: Checkout
113113
uses: actions/checkout@v4
114114

115-
- name: Install MySQL / GD / ZIP PHP extensions
115+
- name: Install MySQL / GD / ZIP / LDAP PHP extensions
116116
run: |
117-
apk add $PHPIZE_DEPS icu-libs icu-dev libpng-dev libzip-dev
117+
apk add $PHPIZE_DEPS icu-libs icu-dev libpng-dev libzip-dev openldap-dev
118118
docker-php-ext-configure intl
119119
docker-php-ext-configure gd
120120
docker-php-ext-configure zip
121-
docker-php-ext-install pdo pdo_mysql intl gd zip
121+
# ext-ldap is optional for Davis (only AUTH_METHOD=LDAP needs it) but the LDAP tests
122+
# skip themselves without it, and we would rather run them
123+
docker-php-ext-install pdo pdo_mysql intl gd zip ldap
122124
123125
- name: Install Composer
124126
run: wget -qO - https://getcomposer.org/installer | php -- --install-dir=/usr/local/bin --filename=composer --quiet

README.md

Lines changed: 4 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -438,24 +438,11 @@ More examples and information [here](https://symfony.com/doc/current/setup/web_s
438438
439439
Web-based protocols like CalDAV and CardDAV can be found using a discovery service. Some clients require that you implement a path prefix to point to the correct location for your service. See [here](https://en.wikipedia.org/wiki/List_of_/.well-known/_services_offered_by_webservers) for more info.
440440
441-
If you use Apache as your webserver, you can enable the redirections with:
441+
Davis answers `/.well-known/caldav` and `/.well-known/carddav` itself and redirects them to its DAV endpoint, so **no web server configuration is needed**. Because the redirect is built from the application's own base path, it also works when Davis is installed in a sub-directory (`https://example.org/davis/`), which a hard-coded `/dav/` rewrite does not.
442442
443-
```apache
444-
RewriteEngine On
445-
RewriteRule ^\.well-known/carddav /dav/ [R=301,L]
446-
RewriteRule ^\.well-known/caldav /dav/ [R=301,L]
447-
```
448-
449-
Make sure that `mod_rewrite` is enabled on your installation beforehand.
450-
451-
If you use Nginx, you can add this to your configuration:
452-
453-
```nginx
454-
location / {
455-
rewrite ^/.well-known/carddav /dav/ redirect;
456-
rewrite ^/.well-known/caldav /dav/ redirect;
457-
}
458-
```
443+
> [!NOTE]
444+
>
445+
> If your web server still rewrites these two paths itself (earlier versions of this README suggested doing so), you can remove those rules: they take precedence over Davis and will send clients to the wrong place on a sub-directory installation.
459446
460447
# 🐳 Dockerized installation
461448

docker/configurations/Caddyfile

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,6 @@
33
}
44

55
:9000 {
6-
# Redirect .well-known
7-
redir /.well-known/caldav /dav/
8-
redir /.well-known/carddav /dav/
9-
106
root * /var/www/davis/public
117
php_fastcgi unix//var/run/php-fpm/php-fpm.sock {
128
# Preserve the original X-Forwarded-Proto from upstream, as it might be HTTPS

docker/configurations/nginx.conf

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,6 @@ server {
1414
root /var/www/davis/public/;
1515
index index.php;
1616

17-
rewrite ^/.well-known/caldav /dav/ redirect;
18-
rewrite ^/.well-known/carddav /dav/ redirect;
19-
2017
charset utf-8;
2118

2219
# Security headers (add `Strict-Transport-Security` once TLS is terminated in front of nginx)

public/.htaccess

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,10 +20,6 @@ DirectoryIndex index.php
2020
<IfModule mod_rewrite.c>
2121
RewriteEngine On
2222

23-
# Add .well-known redirections
24-
RewriteRule ^\.well-known/carddav /dav/ [R=301,L]
25-
RewriteRule ^\.well-known/caldav /dav/ [R=301,L]
26-
2723
# Determine the RewriteBase automatically and set it as environment variable.
2824
# If you are using Apache aliases to do mass virtual hosting or installed the
2925
# project in a subdirectory, the base path will be prepended to allow proper

src/Controller/Api/ApiController.php

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
use App\Entity\CalendarSubscription;
88
use App\Entity\Principal;
99
use App\Entity\User;
10+
use App\Services\Utils;
1011
use Doctrine\Persistence\ManagerRegistry;
1112
use Sabre\DAV\Sharing\Plugin as SharingPlugin;
1213
use Symfony\Bundle\FrameworkBundle\Controller\AbstractController;
@@ -26,7 +27,7 @@ class ApiController extends AbstractController
2627
*/
2728
private function validateUsername(string $username): bool
2829
{
29-
return !empty($username) && is_string($username) && !preg_match('/[^a-zA-Z0-9_.@-]/', $username);
30+
return Utils::isValidUsername($username);
3031
}
3132

3233
/**

src/Controller/DAVController.php

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -312,6 +312,20 @@ private function initExceptionListener()
312312
});
313313
}
314314

315+
/**
316+
* Service discovery (RFC 6764).
317+
*
318+
* This lives in the application rather than in each web server's configuration so that
319+
* the redirect is built from the real base path: a hard-coded `/dav/` sends clients to
320+
* the wrong place whenever Davis is installed under a sub-directory.
321+
*/
322+
#[Route('/.well-known/caldav', name: 'well_known_caldav')]
323+
#[Route('/.well-known/carddav', name: 'well_known_carddav')]
324+
public function wellKnown(): Response
325+
{
326+
return $this->redirectToRoute('dav', ['path' => ''], Response::HTTP_MOVED_PERMANENTLY);
327+
}
328+
315329
#[Route('/dav/{path}', name: 'dav', requirements: ['path' => '.*'])]
316330
public function dav(Request $request, ?string $path, ?Profiler $profiler = null)
317331
{
@@ -327,7 +341,16 @@ public function dav(Request $request, ?string $path, ?Profiler $profiler = null)
327341

328342
// Adapted from CorePlugin's httpOptions()
329343
// https://github.com/sabre-io/dav/blob/master/lib/DAV/CorePlugin.php#L210
330-
$methods = $this->server->getAllowedMethods('');
344+
//
345+
// The methods depend on the node being asked about: MKCALENDAR, for instance, is
346+
// only offered inside a calendar home. Answering for the root instead of the
347+
// requested path told every client the same, incomplete story.
348+
try {
349+
$methods = $this->server->getAllowedMethods($path ?? '');
350+
} catch (\Throwable $e) {
351+
// An unresolvable path should still get a usable answer
352+
$methods = $this->server->getAllowedMethods('');
353+
}
331354

332355
$response->headers->set('Allow', strtoupper(implode(', ', $methods)));
333356
$features = ['1', '3', 'extended-mkcol'];

src/Entity/User.php

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,18 @@ class User
1818
#[ORM\Column(type: 'integer')]
1919
private $id;
2020

21+
/**
22+
* A username ends up in the principal URI (`principals/<username>`), so it must not carry
23+
* anything that would change that path's structure. Letters, digits and `_ . @ + ' -` are allowed:
24+
* the punctuation is what shows up in mail-derived login names. Enforced when a user is created; existing
25+
* accounts are left alone so that an odd username created before this rule stays editable.
26+
*/
27+
public const USERNAME_PATTERN = '/^[a-zA-Z0-9_.@+\'-]+$/';
28+
2129
#[ORM\Column(type: 'string', length: 255, unique: true)]
2230
#[Assert\NotBlank]
31+
#[Assert\Length(max: 255, groups: ['creation'])]
32+
#[Assert\Regex(pattern: self::USERNAME_PATTERN, message: 'form.username.invalid', groups: ['creation'])]
2333
private $username;
2434

2535
#[ORM\Column(name: 'digesta1', type: 'string', length: 255)]

src/Form/UserType.php

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
use Symfony\Component\Form\Extension\Core\Type\SubmitType;
1212
use Symfony\Component\Form\Extension\Core\Type\TextType;
1313
use Symfony\Component\Form\FormBuilderInterface;
14+
use Symfony\Component\Form\FormInterface;
1415
use Symfony\Component\OptionsResolver\OptionsResolver;
1516

1617
class UserType extends AbstractType
@@ -55,6 +56,11 @@ public function configureOptions(OptionsResolver $resolver): void
5556
$resolver->setDefaults([
5657
'new' => false,
5758
'data_class' => User::class,
59+
// The username rule only applies to new accounts: the field is disabled when editing,
60+
// and an account created before the rule (or by LDAP/IMAP) must stay editable.
61+
'validation_groups' => static fn (FormInterface $form): array => $form->getConfig()->getOption('new')
62+
? ['Default', 'creation']
63+
: ['Default'],
5864
]);
5965
}
6066
}

src/Services/AbstractAuth.php

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@
33
namespace App\Services;
44

55
use Sabre\DAV\Auth\Backend\AbstractBasic;
6+
use Sabre\HTTP\RequestInterface;
7+
use Sabre\HTTP\ResponseInterface;
68

79
/**
810
* Common base for the HTTP Basic authentication backends (internal, IMAP, LDAP).
@@ -12,24 +14,78 @@
1214
* and an empty password means an *unauthenticated bind* for LDAP servers, which Active
1315
* Directory (and OpenLDAP with `allow bind_anon_cred`) answers with success, i.e. it
1416
* would log the caller in as any user.
17+
*
18+
* It also rejects usernames that would not survive being put in a principal URI. sabre
19+
* derives the principal from the login name (`principals/<username>`), so a name containing
20+
* a slash would address a different, possibly existing, node: `alice/calendar-proxy-write`
21+
* is exactly the URI Davis uses for alice's delegation proxy. Only structural characters are
22+
* refused here, not the stricter set required when creating an account, so that an unusual
23+
* but working username keeps authenticating.
1524
*/
1625
abstract class AbstractAuth extends AbstractBasic
1726
{
27+
/**
28+
* The username as the backend spells it, when that differs from what the client sent.
29+
*/
30+
private ?string $canonicalUsername = null;
31+
1832
/**
1933
* @param string $username
2034
* @param string $password
2135
*/
2236
final protected function validateUserPass($username, $password): bool
2337
{
38+
$this->canonicalUsername = null;
39+
2440
if (!is_string($username) || !is_string($password) || '' === $username || '' === $password) {
2541
return false;
2642
}
2743

44+
if (self::breaksPrincipalUri($username)) {
45+
return false;
46+
}
47+
2848
return $this->checkCredentials($username, $password);
2949
}
3050

3151
/**
3252
* Validates a non-empty username and password against the backend.
3353
*/
3454
abstract protected function checkCredentials(string $username, string $password): bool;
55+
56+
/**
57+
* Backends call this when the directory spells the username differently from what the
58+
* client sent — LDAP matches `ALICE` against `uid=alice` quite happily. The principal is
59+
* then built from that spelling instead, so the login, the account and the principal URI
60+
* cannot drift apart and produce a second, empty account.
61+
*/
62+
protected function setCanonicalUsername(string $username): void
63+
{
64+
// It ends up in a principal URI like any other username
65+
if ('' !== $username && !self::breaksPrincipalUri($username)) {
66+
$this->canonicalUsername = $username;
67+
}
68+
}
69+
70+
/**
71+
* @return array{0: bool, 1: string}
72+
*/
73+
public function check(RequestInterface $request, ResponseInterface $response)
74+
{
75+
$result = parent::check($request, $response);
76+
77+
if (true === $result[0] && null !== $this->canonicalUsername) {
78+
return [true, $this->principalPrefix.$this->canonicalUsername];
79+
}
80+
81+
return $result;
82+
}
83+
84+
private static function breaksPrincipalUri(string $username): bool
85+
{
86+
// Anything that would change the shape of `principals/<username>`:
87+
// [/\\] a forward or back slash, which would add a path segment
88+
// [\x00-\x20\x7f] any control character, plus space (0x20) and DEL (0x7f)
89+
return 1 === preg_match('~[/\\\\]|[\\x00-\\x20\\x7f]~', $username);
90+
}
3591
}

0 commit comments

Comments
 (0)