mstilkerich / rcmcarddav

CardDAV plugin for RoundCube Webmailer
GNU General Public License v2.0
257 stars 81 forks source link

Support preemptive basic authentication as advanced option #407

Closed dilyanpalauzov closed 1 year ago

dilyanpalauzov commented 1 year ago

The CardDAV server https://cal.aegee.org does provide valid answer, when no username is submitted, and when the username matches the password. When the username does not match the password, the server returns 401.

In Preferences → CardDAV → I create a new addressbook:

I attach with strace to the php-process.

Then I click on Record/Save/Search in RoundCube/CardDAV. RoundCube/CardDAV/mstilkerich/carddavclient do send PROPFIND requests, but these have no username/password. Thus the returned addressbooks are not for the logged in user.

I do use above http://127.0.0.4, but you can try it with https://cal.aegee.org/ . In the latter case strace does not show the communication in clear text.

Please alter rcmcarddav to always send username to the server. Then each of my users can obtain the public and the private addressbooks.

dilyanpalauzov commented 1 year ago

For testing: username aaa domain aegee.org secret abc . When the setup is correct the user agent shall see two addressbooks: Bodies and Default.

mstilkerich commented 1 year ago

Hello,

your server is replying to an unauthenticated requested for the DAV:current-user-principal property with a principal URI for an anonymous user. This appears to be not in line with RFC5397, according to which this property represents

URL for the currently authenticated user's principal resource on the server

That is, it is not expected that a URI is returned to an unauthenticated request for this property. In fact, RFC5397 explicitly specifies that in case of lacking or failed authentication the response must include a DAV:unauthenticated element instead of an URI (DAV:href element).

Furthermore, RFC6764 explicitly requires the server to require authentication when querying the DAV:current-user-principal property:

Servers MUST force authentication for "PROPFIND" requests that retrieve the DAV:current-user-principal property to ensure that the value of the DAV:current-user-principal property returned corresponds to the principal-URL of the user making the request.

The authentication challenge by the server is required for the client to know the supported authentication schemes. It appears to me that your use case of unauthenticated access to a public addressbook is not considered in the service-discovery mechanism described in RFC6764.

What would be possible with rcmcarddav is to use admin presets where you explicitly specify the URLs of addressbooks and disable the discovery mechanism. The URLs could include placeholders, particularly the user identifier. Would this be an option for you?

dilyanpalauzov commented 1 year ago

Returning vCards without authentication is a valid use case, as is returning contacts from LDAP without authentication.

The purpose of returning valid href on DAV:current-user-principal for unauthenticated requests is to permit the user-agent to find the addressbook-home-sets and then the public addressbooks.

Please change rcmcarddav to always send credentials when doing WebDAV requests.

In the cases where the bootstraping is not configured properly server side, and the user provides as input a URL that is either a principal-url, an addressbook-home-set, or and addresbook-URL, rcmcarddav shall try to get data from the URLs, instead of complaining that the principal-url cannot be found. Currently if I enter my addressbook-homeset URL as input, it is ignored, and instead the addressbook-home-set of the anonymous user is used.

What would be possible with rcmcarddav is to use admin presets where you explicitly specify the URLs of addressbooks and disable the discovery mechanism. The URLs could include placeholders, particularly the user identifier. Would this be an option for you?

A user can have many addressbooks, one private addressbook-home-set, and see one public addressbook-home-set. I cannot enter in the preset the URLs of all addressbooks, as these can be for each user different in terms of address and amount. I can enter in the preset the addressbook-home-set, e.g. https://cal.aegee.org/dav/addressbooks/user/%u. Then discovery shall be done using that URL as start. I could also enter as URL in the preset https://cal.aegee.org/dav/principals/user/%u/ in which case this URL shall be used as principal-URL for the subsequent discovery.

Since a user can have many private addressbooks, it is not possible to specify them in a preset, disable discovery and expect that users see all their addressbooks.

To reduce the amount of WebDAV requests, when a URL is provided in a preset, it shall be possible to specify if this is a principal-URL, an addressbook-home-set URL or an addressbook.

mstilkerich commented 1 year ago

You will need a modified version of mstilkerich/carddavclient for your setup to work, which I have put to a branch here: https://github.com/mstilkerich/carddavclient/tree/aegee

No changes in rcmcarddav itself are needed.

Apart from the preemptive auth, it is also required to support multiple addressbook home URIs, which is also included in the above branch.

I don't know when / if I will integrate this into the master. As I wrote above, returning a principal URI for an unauthenticated request is not in compliance with RFCs. Properly doing it and making it available as an option in rcmcarddav incl. testing would be quite a bit of effort.

dilyanpalauzov commented 1 year ago

With the patch applied and this preset:

$prefs['AEGEE Addressbook (CardDAV)'] = [
    // required attributes                                                                                                              
    'name'         =>  'AEGEE Addressbook (CardDAV)',
    'url'          =>  'https://cal.aegee.org/dav/addressbooks/', // and 'http://127.0.0.4/dav/addressbooks/' also works

    // required attributes unless passwordless authentication is used (Kerberos)                                                        
    'username'     =>  '%u',
    'password'     =>  '%p',

    // optional attributes                                                                                                              
    'active'       =>  true,
    'readonly'     =>  false,
    'refresh_time' => '03', //<Refresh Time in Hours, Format HH[:MM[:SS]]>',                                                            
    'rediscover_mode' => 'fulldiscovery', // '<One of: none|fulldiscovery',                                                             

    // attributes that are fixed (i.e., not editable by the user) and auto-updated for this preset                                      
    // 'fixed'        =>  [ < 0 or more of the other attribute keys > ],                                                                

    // always require these attributes, even for addressbook view                                                                       
    'require_always' => ['email'],

    // hide this preset from CardDAV preferences section so users can't even see it                                                     
    'hide' => false,
];

all addressbooks are shown by default to the user. Thanks!

Since some addressbooks are read-only, and others read-write, the CardDAV client shall detect whether each displayed addressbook is read-only for the current user.

Provided that returning principal URL for unauthenticated request is supposed to return the unauthenticated-constant, why does the CardDAV user agent, before the patch, make an unauthenticated request to obtain the unauthenticated-constant, instead of sending authenticated PROPFIND when requesting current-user-principle? This would avoid the question if the unauthenticated question is valid or not.

Please extend the rediscover_mode with a value “principal-url”, where the value of URL is interpreted as principal-url and ./.well-known/carddav is not queried.

In any case your patch already handles the case of returned multiple addressbooks-home-sets, so perhaps you can for the beginning include upstream just this change?

dilyanpalauzov commented 1 year ago

N.B. Preemptive basic auth for every request reduces the total numbers of requests needed and speeds up the discovery process. Without preemptive auth some requests get 401 answer and have to be repeated, thus there are more requests in total.

dilyanpalauzov commented 1 year ago

You will need a modified version of mstilkerich/carddavclient for your setup to work, which I have put to a branch here: https://github.com/mstilkerich/carddavclient/tree/aegee

As both the aforementioned branch and the release branch contain (different) code to accommodate accounts with multiple homesets, can you please update https://github.com/mstilkerich/carddavclient/tree/aegee to include only the preemptive authentication?

mstilkerich commented 1 year ago

What’s the problem? The branch should continue to work just fine. I guess you do not install this branch using composer, do you?

dilyanpalauzov commented 1 year ago

The problem is that I want to use the lastest version of carddavclient, which happens to get installed on new roundcube-1.6.2 systems.

In Roundcube 1.6.2 I install rcmcarddav with composer require --update-no-dev -o "roundcube/carddav:*". It installs indirectly carddavclient-1.4.0. Then in vendor/mstilkerich I apply the changeset https://github.com/mstilkerich/carddavclient/commit/6a012fdfc16a688a3dfff7ffdf0eca96e449c47d.patch but only the modifications in src/HttpClientAdapterGuzzle.php, since the installed version 1.4.0 is supposed already to support servers with multiple addressbooks home locations.

I use the following configuration file, which you can also use with user aaa or aaa@aegee.org and password abc.

It does not work.

In the directory vendor/mstilkerich I rename carddavclient to carddavclient.orig, I download https://github.com/mstilkerich/carddavclient/archive/refs/tags/v1.3.0.tar.gz, unpack, rename carddavclient-1.3.0 to carddavclient, and apply the changeset https://github.com/mstilkerich/carddavclient/commit/6a012fdfc16a688a3dfff7ffdf0eca96e449c47d.patch .

Now it works with the configuration file below.

It would be good if that magic :443 disappears from the URL for matching.

<?php

$prefs['_GLOBAL']['pwstore_scheme'] = 'plain';

$prefs['_GLOBAL']['loglevel'] = \Psr\Log\LogLevel::WARNING;
$prefs['_GLOBAL']['loglevel_http'] = \Psr\Log\LogLevel::ERROR;

$prefs['_GLOBAL']['collected_recipients'] = [
  'preset' => 'CardDAV', 
  'matchurl' => '#https://mail.aegee.org:443/dav/addressbooks/user/%l/Default/#'
];
$prefs['_GLOBAL']['collected_senders'] = [
  'preset' => 'CardDAV', 
  'matchurl' => '#https://mail.aegee.org:443/dav/addressbooks/user/%l/Default/#'
];

$prefs['CardDAV'] = [
    'accountname'     =>  'acc1',
    'username'        => '%u',
    'password'        => '%p',
    'discovery_url'   =>  'https://mail.aegee.org/dav/addressbooks/',
    'rediscover_time' => '00:01',
    'fixed'           =>  ['discovery_url', 'username', 'password'],
    'name'            => '%N',
];
mstilkerich commented 1 year ago

You can simply replace the entire carddavclient directory in your install with the one from the aegee branch. There is nothing new that you would need in 1.4.0, neither have any bugs been fixed. I will add options for preemptive auth to rcmcarddav (on this ticket) and when released you will not have to use workarounds any longer.