Customize presentation of accept/decline buttons in iMip mail #12392

brad2014 · 2018-11-10T10:38:19Z

Fix Issue CalDAV Mail Invitations: Bogus Invite/Accept upon deleted person #11230
Only present accept/decline button links in iMip mail for REQUEST, not CANCEL or others.
Fix Issue Support iMip emails that don't link to server, for nextcloud servers on a private network #12156
Implement config setting "dav.invitation_link_recipients", to control
which invitation recipients see accept/decline button links. The
default, for public internet facing servers, is to always include
them. For a server on a private intranet, this setting can be set
to the email addresses or email domains of users whose browsers can
access the nextcloud server referenced by those accept/decline
button links. It can also be set to "false" to exclude the links
from all requests.

Signed-off-by: Brad Rubenstein brad@wbr.tech

kesselb · 2018-11-10T12:25:34Z

config/config.sample.php

+*   'dav.invitation_link_recipients' => false,
+*
+*/
+'dav.invitation_link_recipients' => '*', // always include accept/reject server links in iMip emails


For nextcloud servers accessible to the public internet, the default
"dav.invitation_link_recipients" value "true" (all recipients) is appropriate.

I guess the default here should be true?

brad2014 · 2018-11-10T16:55:10Z

💥 Yes, you are correct, thanks! 💥 Fixed.

brad2014 · 2018-11-10T17:05:33Z

apps/dav/lib/CalDAV/Schedule/IMipPlugin.php

@@ -144,11 +144,11 @@ public function schedule(Message $iTipMessage) {

 		$summary = $iTipMessage->message->VEVENT->SUMMARY;

-		if (parse_url($iTipMessage->sender, PHP_URL_SCHEME) !== 'mailto') {
+		if (strcasecmp(parse_url($iTipMessage->sender, PHP_URL_SCHEME), 'mailto') !== 0) {


Note: One more place where mailto scheme comparison should be case insensitive (if the event contained MAILTO, then it would not have properly been detected). This is related to the fix nextcloud/calendar@82fc0a9 by @georgehrke.

brad2014 · 2018-11-12T16:20:39Z

Another note: It occurs to me that RFC5545 defaults ATTENDEE rsvpparm to RSVP=FALSE. Perhaps the invite buttons should only be presented when (a) the attendee has a mailto, and (b) the method is REQUEST, and (c) RSVP=TRUE. (The examples in the RFC suggest it would be set false for optional attendees and non-individuals, e.g. resources). The code does not currently look at the RSVP parameter, as far as I can tell. May I ask @georgehrke or @MorrisJobke for advice? I'm happy to put it in if it seems the right thing to do.

georgehrke · 2018-11-12T16:26:29Z

Hi @brad2014,

Thx for your pull-request!
Can you please add unit-tests for your changes?

Perhaps the invite buttons should only be presented when (a) the attendee has a mailto,

If the attendee doesn't have a mailto, we can't send this email anyway, so that's already the case.

the method is REQUEST

Will be done by this pull-request, correct?

The code does not currently look at the RSVP parameter, as far as I can tell

Most of this is not done in Nextcloud directly, but in Sabre/Dav.
But skimming over https://github.com/nextcloud/3rdparty/blob/214c4155f587f5178d792fe4a839044bdc9982f1/sabre/vobject/lib/ITip/Broker.php this doesn't seem to be the case indeed.

brad2014 · 2018-11-12T16:47:45Z

Most of this is not done in Nextcloud directly, but in Sabre/Dav.
But skimming over https://github.com/nextcloud/3rdparty/blob/214c4155f587f5178d792fe4a839044bdc9982f1/sabre/vobject/lib/ITip/Broker.php this doesn't seem to be the case indeed.

My notes: The sabre ITip code properly passes the client's RSVP parameter through unchanged, except when processing an incoming reply (to remove the RSVP=TRUE parameter when an RSVP has been received). Other than that, it is up to the client to decide when RSVP's are desired (e.g. apps/calendar (at the UI) or server/dav (by nextcloud policy) chooses whether rsvp's are desired from "required" or "optional" participants, and, on the other end, the server/dav/lib/CalDAV/Schedule/IMipPlugin.php must decide what to present in emails when the organizer (or nextcloud by policy) has set RSVP to true or false when they created the event that generates the REQUEST.

brad2014 · 2018-11-12T17:07:19Z

Thx for your pull-request!
Can you please add unit-tests for your changes?

I need to learn how to use phpUnit. :-)

I'll look for a doc that tells me how to set up my local machine to run nextcloud tests (pointers welcome).

brad2014 · 2019-02-26T03:38:14Z

@georgehrke or @MorrisJobke : I'm looking at the existing tests, to find a model for a new unit test, and I think I need some guidance. I'm not sure where to ask, so I'll ask here. I hope you don't mind.

I read unit testing. I thought I'd first try to get testing working. On a clean Fedora-28 VM with the nextcloud dependencies installed, and the latest phpunit. But when I run ./autotest.sh, I get

$ ./autotest.sh
Using PHP executable /usr/bin/php
PHP Warning:  require_once(/vagrant/server/3rdparty/autoload.php): failed to open stream: No such file or directory in /vagrant/server/build/OCPSinceChecker.php on line 23
PHP Fatal error:  require_once(): Failed opening required '/vagrant/server/3rdparty/autoload.php' (include_path='.:/usr/share/pear:/usr/share/php') in /vagrant/server/build/OCPSinceChecker.php on line 23

What am I missing?

Any suggestions on how to make a unit test for this or where to put it in the tree? Are there any existing caldav tests I can use for a model?

MorrisJobke · 2019-02-26T08:28:20Z

What am I missing?

Did you ran git submodule update --init to initialize the 3rdparty git submodule?

brad2014 · 2019-02-26T16:28:40Z

Thanks.

Did you ran git submodule update --init to initialize the 3rdparty git submodule?

Thanks. Not sure how I missed it - the docs are clear.

brad2014 · 2019-02-27T17:43:00Z

1. Any suggestions on how to make a unit test for this or where to put it in the tree?  Are there any existing caldav tests I can use for a model?

I think I've figured it out. I will write some unit tests shortly.

Fix Issue #11230 Only present accept/decline button links in iMip mail for REQUEST, not CANCEL or others. Fix Issue #12156 Implement config setting "dav.invitation_link_recipients", to control which invitation recipients see accept/decline button links. The default, for public internet facing servers, is to always include them. For a server on a private intranet, this setting can be set to the email addresses or email domains of users whose browsers can access the nextcloud server referenced by those accept/decline button links. It can also be set to "false" to exclude the links from all requests. Signed-off-by: Brad Rubenstein <brad@wbr.tech>

Signed-off-by: Brad Rubenstein <brad@wbr.tech>

If RSVP=TRUE parameter is FALSE or absent for an ATTENDEE, then do no present accept/decline buttons. The organizer isn't asking for an RSVP. Signed-off-by: Brad Rubenstein <brad@wbr.tech>

My oops. The comparisons, which are copied from the IMipPlugin shipped with sabre-io/dav, do not need to be case insensitive because the sender and recipient names are normalized by sabre, (see calls to getNormalizedValue in voboject/lib/ITip/Broker.php). Signed-off-by: Brad Rubenstein <brad@wbr.tech>

Existing tests required modification to correctly mock up the new config parameter fetch, and to set the RSVP flag for attendees (since the test was detecting token generation, and we no longer generate tokens when no RSVP is requested by the client or sent by nextcloud). Signed-off-by: Brad Rubenstein <brad@wbr.tech>

Signed-off-by: Brad Rubenstein <brad@wbr.tech>

brad2014 · 2019-02-28T01:59:35Z

@georgehrke, I have added unit tests per your suggestion - In the server-generated iMIP email, I'm using link token generation as a surrogate for whether buttons and links are added (the test doesn't actually look at the generated HTML - perhaps it should).

I also did the following:

Rebased my changes (trivially) to the top of the master branch.
Moved all the repeated in apps/dav/tests/unit/CalDAV/Schedule/IMipPluginTest.php into a phpUnit setUp function. The minimal change to the test file (to add the new tests and fix the old ones) is 1dbda26, and the refactor follows in 1a29239. (It is 43% DRYer)

The tests pass, I should add, pass 😄.

brad2014 · 2019-02-28T02:00:50Z

[This is my first pull request for nextcloud - if you all have any suggestions or concerns about my code or process, I'm very interested in hearing them.]

MorrisJobke · 2019-03-06T09:01:37Z

Hi @brad2014 - sorry that we didn't had time to look into this for the Nextcloud 16 milestone. We were quite busy with other tasks. We still appreciate the work you put into this, but the freeze for Nextcloud 16 is active since last Friday and I will put this into the Nextcloud 17 bucket. I hope that is okay for you.

apps/dav/lib/CalDAV/Schedule/IMipPlugin.php

@georgehrke

Per @georgehrke change request for PR #12392, instead of setting dav.invitation_link_recipients in the system config.php file, we set it in the database table oc_appconfig. Furthermore, the value of the config variable is always a string: 'yes' to include links in imip mail, 'no' to exclude them, or a comma-separated list of email addresses and/or domains for which they should be included. If not specified in oc_appconfig, the default is 'yes'. Signed-off-by: brad2014 <brad2014@users.noreply.github.com>

brad2014 · 2019-07-31T19:59:34Z

@georgehrke I have addressed all your review comments on the branch, but I'm not sure if the next step is for me to do something to remove the "Changes Requested" blocker, or if that's something you do after additional review. Please advise. Thank you!

georgehrke · 2019-07-31T20:10:02Z

@brad2014 The changes requested blocker will go after another review. It's already late in Germany, but i will look into this first thing tomorrow morning. :)

brad2014 · 2019-07-31T23:34:40Z

Thank you (didn't mean to rush anyone, just wasn't sure if I needed to do anything).

georgehrke

👍

Let me check why CI didn't start

@georgehrke

Per @georgehrke change request for PR #12392, instead of setting dav.invitation_link_recipients in the system config.php file, we set it in the database table oc_appconfig. Furthermore, the value of the config variable is always a string: 'yes' to include links in imip mail, 'no' to exclude them, or a comma-separated list of email addresses and/or domains for which they should be included. If not specified in oc_appconfig, the default is 'yes'. Signed-off-by: brad2014 <brad2014@users.noreply.github.com>

georgehrke · 2019-08-01T08:46:15Z

https://drone.nextcloud.com/nextcloud/server/20578

@georgehrke

Per @georgehrke change request for PR #12392, instead of setting dav.invitation_link_recipients in the system config.php file, we set it in the database table oc_appconfig. Furthermore, the value of the config variable is always a string: 'yes' to include links in imip mail, 'no' to exclude them, or a comma-separated list of email addresses and/or domains for which they should be included. If not specified in oc_appconfig, the default is 'yes'. Signed-off-by: brad2014 <brad2014@users.noreply.github.com>

georgehrke · 2019-08-14T14:58:51Z

CI is happy: https://drone.nextcloud.com/nextcloud/server/20598

georgehrke · 2019-08-14T14:59:27Z

please review @rullzer @juliushaertl @ChristophWurst :)

brad2014 requested a review from georgehrke November 10, 2018 11:54

kesselb reviewed Nov 10, 2018

View reviewed changes

brad2014 commented Nov 10, 2018

View reviewed changes

brad2014 self-assigned this Nov 12, 2018

brad2014 added the feature: dav label Nov 12, 2018

MorrisJobke added this to the Nextcloud 16 milestone Nov 12, 2018

MorrisJobke added enhancement 3. to review Waiting for reviews labels Nov 12, 2018

brad2014 requested a review from rullzer November 12, 2018 16:29

MorrisJobke requested review from skjnldsv and blizzz February 26, 2019 16:38

Brad Rubenstein added 6 commits February 28, 2019 01:41

corrected sample config

15400dd

Signed-off-by: Brad Rubenstein <brad@wbr.tech>

Respect RSVP parameter for attendees when adding accept/decline buttons.

6421e30

If RSVP=TRUE parameter is FALSE or absent for an ATTENDEE, then do no present accept/decline buttons. The organizer isn't asking for an RSVP. Signed-off-by: Brad Rubenstein <brad@wbr.tech>

IMipPlugin: DRY up - move common test set-up into setUp function

1a29239

Signed-off-by: Brad Rubenstein <brad@wbr.tech>

MorrisJobke mentioned this pull request Mar 4, 2019

16.0.0 Alpha 1 #14501

Merged

45 tasks

MorrisJobke mentioned this pull request Jul 17, 2019

17.0.0 Beta 1 #16408

Merged

28 tasks

MorrisJobke modified the milestones: Nextcloud 17, Nextcloud 18 Jul 17, 2019

brad2014 commented Jul 18, 2019

View reviewed changes

apps/dav/lib/CalDAV/Schedule/IMipPlugin.php Show resolved Hide resolved

brad2014 requested review from georgehrke and kesselb July 18, 2019 00:49

georgehrke approved these changes Aug 1, 2019

View reviewed changes

georgehrke mentioned this pull request Aug 1, 2019

Feature/noid/invitation buttons request only this is only for ci #16625

Closed

georgehrke requested review from juliusknorr and ChristophWurst August 14, 2019 14:59

rullzer approved these changes Aug 15, 2019

View reviewed changes

rullzer modified the milestones: Nextcloud 18, Nextcloud 17 Aug 15, 2019

rullzer merged commit 954290a into nextcloud:master Aug 15, 2019

This was referenced Sep 11, 2019

Support iMip emails that don't link to server, for nextcloud servers on a private network #12156

Closed

CalDAV Mail Invitations: Bogus Invite/Accept upon deleted person #11230

Closed

Remove accept/decline buttons in invitation canceled emails #14791

Closed

brad2014 deleted the feature/properly-present-buttons-from-imip-mails branch September 14, 2019 18:07

brad2014 mentioned this pull request Sep 14, 2019

Minor improvements to iMip mail message text #12391

Closed

brad2014 mentioned this pull request Oct 9, 2019

iMIP email improvements (take 2) #17456

Merged

mrtumnus mentioned this pull request May 24, 2021

Calendar invitation accept/decline buttons missing #21189

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Customize presentation of accept/decline buttons in iMip mail #12392

Customize presentation of accept/decline buttons in iMip mail #12392

brad2014 commented Nov 10, 2018

kesselb Nov 10, 2018

brad2014 commented Nov 10, 2018 •

edited

Loading

brad2014 Nov 10, 2018 •

edited

Loading

brad2014 commented Nov 12, 2018

georgehrke commented Nov 12, 2018

brad2014 commented Nov 12, 2018 •

edited

Loading

brad2014 commented Nov 12, 2018

brad2014 commented Feb 26, 2019

MorrisJobke commented Feb 26, 2019

brad2014 commented Feb 26, 2019

brad2014 commented Feb 27, 2019

brad2014 commented Feb 28, 2019 •

edited

Loading

brad2014 commented Feb 28, 2019

MorrisJobke commented Mar 6, 2019

brad2014 commented Jul 31, 2019

georgehrke commented Jul 31, 2019

brad2014 commented Jul 31, 2019

georgehrke left a comment

georgehrke commented Aug 1, 2019

georgehrke commented Aug 14, 2019

georgehrke commented Aug 14, 2019

Customize presentation of accept/decline buttons in iMip mail #12392

Customize presentation of accept/decline buttons in iMip mail #12392

Conversation

brad2014 commented Nov 10, 2018

kesselb Nov 10, 2018

Choose a reason for hiding this comment

brad2014 commented Nov 10, 2018 • edited Loading

brad2014 Nov 10, 2018 • edited Loading

Choose a reason for hiding this comment

brad2014 commented Nov 12, 2018

georgehrke commented Nov 12, 2018

brad2014 commented Nov 12, 2018 • edited Loading

brad2014 commented Nov 12, 2018

brad2014 commented Feb 26, 2019

MorrisJobke commented Feb 26, 2019

brad2014 commented Feb 26, 2019

brad2014 commented Feb 27, 2019

brad2014 commented Feb 28, 2019 • edited Loading

brad2014 commented Feb 28, 2019

MorrisJobke commented Mar 6, 2019

brad2014 commented Jul 31, 2019

georgehrke commented Jul 31, 2019

brad2014 commented Jul 31, 2019

georgehrke left a comment

Choose a reason for hiding this comment

georgehrke commented Aug 1, 2019

georgehrke commented Aug 14, 2019

georgehrke commented Aug 14, 2019

brad2014 commented Nov 10, 2018 •

edited

Loading

brad2014 Nov 10, 2018 •

edited

Loading

brad2014 commented Nov 12, 2018 •

edited

Loading

brad2014 commented Feb 28, 2019 •

edited

Loading