Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feature: set $config['collected_recipients'] and $config['collected_senders'] to carddav address books by default #383

Closed
chaos-prevails opened this issue May 18, 2022 · 7 comments

Comments

@chaos-prevails
Copy link

chaos-prevails commented May 18, 2022

Hello,

I'm using latest RC1.5, and we have 2 default address books for all users:

  1. Personal address book for each users (which should be used for collected recipients)
  2. Staff address book unique for all users (which should be used for collected senders: Trusted senders)

By default, when using SQL address books, to use specific address books to collect recipients, or trust senders, this is "solved" automatically by setting both parameters (collected_recipients, and collected_senders) to true.

Is there maybe a known workaround to have the same functionality when using carddav address books? Right now, I have to manually set both settings via the roundcube webinterface, and the change of setting shows up as "carddav_X" the roundcube parameter field (X being the ID in carddav_addressbooks).

  1. I've already disabled all other address books as described here Disable the roundcube contacts pannel #372
  2. I would like to automatically point (at least) the "collect recipients", ideally also the "trusted senders" to the 2 different carddav address books

If there is no current workaround, what would it need for the carddav plugin to make something like this possible? I think it might be quite common to have those address books (one for staff members, e.g. read-only, and one personal ones to collect email addresses of outgoing email), and setting them by default saves additional config needs per user.

  1. for (pre-configured) address books, new parameters like "collected_recipient_name" or "collected_senders_name" (specifying an identifier (or an array of identifiers) to find the address book name (or URL, or both) which should be used for collecting recipients/trusted senders (I think in many cases the name/URL of those address books would always be very similar containing words like "staff", "personell", "global" or "collected", or "personal" in it, or a specific fixed ID in the URL for one shared address book)
  2. on roundcube side, the parameter collected_recipients, and collected_senders should accept something like "carddav" (update of the documentation)
  3. Whenever a discovery of address books is done, and above roundcube parameters are set to "carddav", the carddav code would need to have a look at all discovered address books of the user, and their names/URLs, and set the roundcube parameter in the table users accordingly (e.g. just get the ID of the address book, and set carddav_X as you can currently do manually).

Manual override would not be possible that way, but maybe that is an acceptable compromise? Otherwise it would need another value somewhere (also in users->parameters?).

That way, no change of code on the roundcube side would be needed, right? Would that be something that is feasible?

@mstilkerich
Copy link
Owner

Hello,

I think this should be possible without any change at the roundcube side, since plugins are able to override roundcube settings.

  • rcmcarddav can set the roundcube options value like it already does for the autocomplete addressbooks
  • The admin configure roundcube to hide the corresponding settings from the preferences panel:
$config['dont_override'] = ['collected_recipients', 'collected_senders'];

However, I believe both addressbooks must be writeable (see here), roundcube will add to the "collected_senders" addressbook the senders of E-Mails where the user allows to download remote resources. Roundcube will also not offer a readonly carddav addressbook for manual configuration by the user in the corresponding preferences.

So I think the key issue here will be the selection mechanism on how to identify the corresponding addressbooks within a preset. I'm thinking in the direction of a pattern match (i.e. allowing placeholders like %u) for the name and URL, or even a regular expression match on those attributes. Questions are how to handle cases where there is no match or several matches possible. For no match, when not setting the option roundcube would resort to the default option configured by the admin. This could be the internal SQL addressbook or also disabling the feature. With several matches, we could either log an error an not set the option in roundcube (behaviour like when no match), or we could set the first (that means an arbitrary) match. But when doing this, we should be sure we choose the same match every time consistently, which might be difficult again (I'm thinking on doing this selection based on config.inc.php setting, without additional DB attributes to mark the addressbook).

@mstilkerich
Copy link
Owner

So I just tried what happens if I set a readonly carddav addressbook from the plugin for roundcube (i.e. an addressbook, that would not be offered by roundcube for the user to select).

In this case, roundcube will nevertheless attempt to store a contact to the addressbook. rcmcarddav will also not check the readonly property (it's really just something that affects roundcube's frontend) and store to the server. That means in case the addressbook is readonly per server-side access permissions, the user will be shown an error message by roundcube that the contact could not be added, otherwise the contact will be created. I therefore think it is better to not allow rcmcarddav to set addressbooks from readonly presets for these two settings, as it violates roundcube's preconditions for these settings.

@chaos-prevails
Copy link
Author

thanks for all the info. Yes, I agree, it wouldn't make sense to choose a readonly address book (I didn't know the process to add trusted senders is "automatic" by allowing remote content).

I think in practical terms, having more than one match for a pattern could be easily prevented by the admin by using the UUID, or full URL (with %u) of the address book for the pattern. Optionally the carddav settings could be hidden, so if the UUID is something readable (e.g. in sogo the default address book is called "personal"), the user could not add additional address books, so there cannot be any interference.

I think the possibility of stating the full URL with a login-dependent sub-string would be the most fail-proof approach, as this should be unique, and users could still be allowed to add other address books.

@mstilkerich
Copy link
Owner

Hello,

I have pushed a prototypic implementation to the branch issue_383_collectedsenders_receivers. Does it satisfy your use case?

Configuration looks like this:

$prefs['_GLOBAL']['collected_recipients'] = [
    'preset'  => 'iCloud',
    'matchname' => '/card/i',
];
$prefs['_GLOBAL']['collected_senders'] = [
    'preset'  => 'iCloud',
    'matchurl' => '#https://p57-contacts.icloud.com:443/\d+/carddavhome/card/#',
];

If both matchname and matchurl are given, both need to match. There must be exactly one match inside the preset, and the preset must not be readonly. Both must be PHP regular expressions. Same placeholders as inside an URL (e.g. %u) are possible inside the regular expressions.

@chaos-prevails
Copy link
Author

chaos-prevails commented May 31, 2022

Hello,

thanks, the automatic set is working quite well! both collected_senders, and collected_recipients are set, and matchname worked perfectly!

However, with matchurl I had troubles. I only got a match if the string did only contain alphanumeric characters, and hyphens. As soon as I used specific special characters (e.g. @, _ or /) (escaped, or not), I didn't get a match, but instead:

[5 ERR] Cannot set special addressbook collected_senders, there are 0 candidates (need: 1)

some example which did work:

$prefs['_GLOBAL']['collected_senders'] = [
    'preset'  => 'SOGo_autodiscovery',
#    'matchname' => '/Staff/i', <-- worked before being commented
    'matchurl' => '/Some-UID-X-1234/', #<-- this part of a URL worked as well 
    'matchurl' => '#SOME-UID-X-1234#', #<-- worked as well
];

These examples however didn't work

$prefs['_GLOBAL']['collected_senders'] = [
    'preset'  => 'SOGo_autodiscovery',
    'matchurl' => '/firstname\_lastname/',  # <-- as soon as `something_` is used, there is no match, however '/\_lastname/' worked
#    'matchurl' => '/https:\/\/mail.mydomain\.net\/SOGo\/dav\/firstname\_lastname\@mydomain\.net\/Contacts\/SOME-UID-X-1234\//', #<-- also with hashtag at either end, there is no match.
'matchurl' => '#https://mail.mydomain.net/SOGo/dav/[email protected]/Contacts/SOME-UID-X-1234/#', #<-- no match either
#    'matchurl' => '/Contacts\/SOME-UID-X-1234/', #<-- no match
#    'matchurl' => '/Contacts/SOME-UID-X-1234/', #<-- no match either
    'matchurl' => '#Contacts/SOME-UID-X-1234#', #<-- no match either
];

I use SOGo as backend, and URLs have the following structure:
https://mail.mydomain.net/SOGo/dav/[email protected]/Contacts/UID-ID-X/

so there are evidently slashes, but also @ and '-'.

Additionally, even if trusted sender was set, and a sender was in this address book, an email from this sender did not automatically show remote content. However, resetting everything to SQL, disabling carddav plugin, setting trusted senders to personal, adding my email to this address book, it still didn't show remote content automatically. So I guess I need another roundcube setting for that.

Regarding collecting recipients, if I sent an email to a new recipient, the recipient email was added to the collected_recipients Carddav book, so that worked perfectly.

As a last remark, it seems the lookup for collected_recipients, and collected_senders is done on each request/page refresh of roundcube. That is great for testing, but in production, I think maybe it would only need to happen once at login after the autodiscovery of all the address books (there shouldn't be any change of available address books within one session, right?)

@mstilkerich
Copy link
Owner

Hello,

concerning the behavior on loading remote images, you have to set the roundcube setting show_images to 3. The default is 0, which means always ask the user.

// Display remote resources (inline images, styles) in HTML messages. Default: 0.
// 0 - Never, always ask
// 1 - Allow from my contacts (all writeable addressbooks + collected senders and recipients)
// 2 - Always allow
// 3 - Allow from trusted senders only
$config['show_images'] = 3;

Concerning the matching, please check exactly how the server reports the URL of the addressbook. For example, with iCloud, the discovered URL will contain the default port 443 for https as shown in my example above. The match needs to work against this URL. You can see the URL in the user interface, or the database. It can be different from what you provided for discovery in the preset.

Concerning special characters in regexes that need escaping see here:

The special regular expression characters are: . \ + * ? [ ^ ] $ ( ) { } = ! < > | : - #

And of course also the character you choose as separator. For URLs, it is practical to not use the slash as separator, that's why I used the hash in the example. The example matchurl I checked in for iCloud works for me. From your example, I don't see a problem with special characters, but rather I don't see that firstname and lastname are included in the URL, so why would you expect a match works?

And concerning your last point: rcmcarddav doesn't keep any data inside the session, it only used the database. The discovery only happens during login, but fetching the available addressbooks and now checking those addressbooks for the collected senders/receivers happens based on the results of this query, so there is no extra DB queries compared to what rcmcarddav does anyway.

@chaos-prevails
Copy link
Author

chaos-prevails commented Jun 1, 2022

Hello,

thanks for all the explanations.

With the parameter $config['show_images'] = 3; I get the option to always allow remote content, and the contact is saved correctly in the trusted senders carddav abook.

Indeed, the URL which is saved in carddav_addressbook is different to the URL which SOGo shows in the frontend to be used in a carddav client.

In SOGo frontend, the "link to Address book" to subscribe to an addressbook shows the following structure:
https://mail.mydomain.net/SOGo/dav/[email protected]/Contacts/SOME-UID-X-1234/

however, in carddav_addressbook the URL is:
https://mail.mydomain.net/SOGo/dav/[email protected]/Contacts/sharername_A_mydomain_D_net_SOME-UID-X-1234/

With the correct URL syntax, hashtags, and %u placeholder, matchurl works, thanks!

Maybe it would make sense to add information in the config file for this parameter that in case an address book which has been shared is used, the exact URL to match should be verified in the RC DB, as the URL is unique, thus different to the subscription URL?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants