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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Calendar invitation response (guests and comment) not processed #23927

Closed
wiswedel opened this issue Nov 6, 2020 · 17 comments 路 Fixed by #35761
Closed

Calendar invitation response (guests and comment) not processed #23927

wiswedel opened this issue Nov 6, 2020 · 17 comments 路 Fixed by #35761
Assignees
Labels
3. to review Waiting for reviews 21-feedback Feedback from 21.x releases 25-feedback bug feature: dav

Comments

@wiswedel
Copy link
Contributor

wiswedel commented Nov 6, 2020

How to use GitHub

  • Please use the 馃憤 reaction to show that you are affected by the same issue.
  • Please don't comment if you have no relevant information to add. It's just extra noise for everyone subscribed to this issue.
  • Subscribe to receive notifications on status change and new comments.

Steps to reproduce

  1. Create a calendar event
  2. Invite somebody by email => invitee receives an email
  3. As the email invitee, click on More options... in the email => browser opens with response page
  4. Pick "accept", fill in a number of guests and leave a comment

calendar accept

  1. Hit Save => see Your attendance was updated successfully.

Expected behaviour

  • The calendar event in the database column oc_calendarobjects.calendardata gets updated with X-RESPONSE-COMMENT and X-NUM-GUESTS
  • The information is shown somewhere on the calendar GUI

Actual behaviour

  • The attendance gets updated, guests and comments never show up anywhere

Server configuration

  • Nextcloud 20.0.1
  • Calendar 2.1.2

Initial evaluation

The feature comes from the server, that's why I'm posting here and not in the calendar repo.

if ($comment) {
$attendee->add('X-RESPONSE-COMMENT', $comment);
$vEvent->add('COMMENT', $comment);
}
if ($guests) {
$attendee->add('X-NUM-GUESTS', $guests);
}

@wiswedel wiswedel added bug feature: dav 0. Needs triage Pending check for reproducibility or if it fits our roadmap labels Nov 6, 2020
@wiswedel wiswedel added 20-feedback 21-feedback Feedback from 21.x releases labels Mar 17, 2021
@szaimen
Copy link
Contributor

szaimen commented Jun 25, 2021

I suppose this is still valid on NC21.0.2?

@ghost
Copy link

ghost commented Jul 25, 2021

This issue has been automatically marked as stale because it has not had recent activity and seems to be missing some essential information. It will be closed if no further activity occurs. Thank you for your contributions.

@ghost ghost added the stale Ticket or PR with no recent activity label Jul 25, 2021
@ghost ghost closed this as completed Aug 8, 2021
@simonmicro
Copy link

I suppose this is still valid on NC21.0.2?

Indeed - this issue should be reopened!

@sbrueck
Copy link

sbrueck commented Jan 18, 2022

Hello all, unfortunately this issue seems to persist. I am using version 22.2.3 with PHP 7.4.3 and MySQL 8.0.27. Unfortunately, data from the "Guests" and "Comments" fields are still not being transferred to the organizer or to the organizer's calendar. Could someone solve this in the meantime?

@ghost ghost removed the stale Ticket or PR with no recent activity label Jan 26, 2022
@tcitworld
Copy link
Member

This is because Sabre doesn't take any custom fields from the iTip message, it just use the email to compare to event attendee existing fields.

https://github.com/nextcloud/3rdparty/blob/41d5f9752c7e0f2b690480a38ef5b9dddc357166/sabre/vobject/lib/ITip/Broker.php#L362-L369

X-RESPONSE-COMMENT and X-NUM-GUESTS are discarded after this.

I don't really know if there's a security issue in accepting attendee-level parameters at this level, so let's start with opening an issue at Sabre. The spec suggests the ATTENDEE property can be changed (from the table talked below), so it might be valid.

About the COMMENT property directly on the VEVENT, I'm again not sure if we can update the property as well.
https://datatracker.ietf.org/doc/html/rfc5546#section-3.2.3

An "Attendee" MAY include a message to the "Organizer" using the "COMMENT" property. For example, if the user indicates tentative acceptance and wants to let the "Organizer" know why, the reason can be expressed in the "COMMENT" property value.

The optional properties listed in the table below (those listed as "0+" or "0 or 1") MUST NOT be changed from those of the original request. If property changes are desired, the "COUNTER" message must be used.
COMMENT | 0+

I'm guessing the spec says you can add a COMMENT property to your REQUEST, but it won't be set as an event property (which kinda makes sense, as you can't know who added it), it's up to the system to forward it to the organizer. Either way Sabre doesn't support this either, so we can remove the following line in any case.

$vEvent->add('COMMENT', $comment);

Should we remove this feature altogether since it won't function until the underneath work has been done? Thoughts @nextcloud/calendar ?

@miaulalala
Copy link
Contributor

We had the same issue for the X-NC-APPOINTMENTS property where it wasn't stored in the oc_calendarobject_props table. I suppose writing it to the ics and then having something to display these properties on the Calendar App side of things parsed from the ics could work. Because that part is fine: the ics will still contain the information when it is created in the iTip message for the InvitationResponseController and will be written to the oc_calendarobjects table like this too.

@ghost
Copy link

ghost commented Feb 26, 2022

This issue has been automatically marked as stale because it has not had recent activity and seems to be missing some essential information. It will be closed if no further activity occurs. Thank you for your contributions.

@ghost ghost added the stale Ticket or PR with no recent activity label Feb 26, 2022
@miaulalala miaulalala removed the stale Ticket or PR with no recent activity label Feb 28, 2022
@colans
Copy link

colans commented Mar 10, 2022

Related to #539

@ghost
Copy link

ghost commented Apr 9, 2022

This issue has been automatically marked as stale because it has not had recent activity and seems to be missing some essential information. It will be closed if no further activity occurs. Thank you for your contributions.

@ghost ghost added the stale Ticket or PR with no recent activity label Apr 9, 2022
@tcitworld tcitworld removed the stale Ticket or PR with no recent activity label Apr 9, 2022
@miaulalala miaulalala self-assigned this Apr 13, 2022
@nextcloud-command
Copy link
Contributor

This issue has been automatically marked as stale because it has not had recent activity and seems to be missing some essential information. It will be closed if no further activity occurs. Thank you for your contributions.

@nextcloud-command nextcloud-command added the stale Ticket or PR with no recent activity label May 16, 2022
@miaulalala miaulalala removed the stale Ticket or PR with no recent activity label May 16, 2022
@nextcloud-command
Copy link
Contributor

This issue has been automatically marked as stale because it has not had recent activity and seems to be missing some essential information. It will be closed if no further activity occurs. Thank you for your contributions.

@nextcloud-command nextcloud-command added the stale Ticket or PR with no recent activity label Jun 16, 2022
@simonmicro
Copy link

Uhmm, I think this is still not fixed? Just commenting to remove the stale label.

@nextcloud-command nextcloud-command removed the stale Ticket or PR with no recent activity label Jun 17, 2022
@nextcloud-command
Copy link
Contributor

This issue has been automatically marked as stale because it has not had recent activity and seems to be missing some essential information. It will be closed if no further activity occurs. Thank you for your contributions.

@nextcloud-command nextcloud-command added the stale Ticket or PR with no recent activity label Jul 17, 2022
@miaulalala miaulalala added 1. to develop Accepted and waiting to be taken care of and removed needs info stale Ticket or PR with no recent activity 0. Needs triage Pending check for reproducibility or if it fits our roadmap labels Jul 25, 2022
@miaulalala
Copy link
Contributor

I have worked on this issue in the sabre code but I'm stuck on how to generate a message in parseMessageForAttendee, so the attendees are also getting the new comments. I think this has to do with the significant change hash.

Upstream PR (still a WIP!) for Sabre is here: https://github.com/sabre-io/vobject/pull/593/files, although after reading @tcitworld 's comment on the security implications and problems with replies, I am not sure it will be accepted.

Also, the export doesn't collect all data stored in the calendarobjects_props table (where the comment now is indeed stored after adding my changes) and does not show them on downloading / exporting the ICS.

I haven't figured out yet where the ICS is built or whether it is an export of the calendarobjects calendardata table (which I find doubtful, as the calendardata string does contain the comments).

Additionally, a frontend display of comments and an option to submit a comment would be nice.

@miaulalala
Copy link
Contributor

After thinking this over, here's the current status:

  1. X-NUM-GUESTS needs to be handled separately as this is not an RFC supported property. Same goes for X-RESPONSE-COMMENT.
  2. Regarding the RFC: COMMENTs should only be sent to and from the ORGANIZER and not passed on to the other ATTENDEEs. cc @kesselb
  3. Upstream fix should be done for the ORGANIZER, even if it isn't supported much (see next point).
  4. The bad news is, we're going to disable the feature (for now). We have no way to display or export the COMMENT at all, our internal export function doesn't include the COMMENT either. It is stored on a DB level (oc_calendarobject_props) when the upstream fix is done but not before.
    Reasoning for removing: Google uses in- house properties and doesn't support COMMENTs only X-RESPONSE-COMMENTs. Thunderbird doesn't support COMMENTS at all. KDE Kalendar doesn't support them either AFAIK. SOGO Groupware may support them, TBD. If anyone wants to tell me how Outlook or Zimbra handles them, let me know. 馃檹 an exported ICS would be much apprechated.
  5. We are also considering refactoring the scheduling plugin so the iTip Broker class can be overwritten more easily. Currently it is instantiated via a call to new Broker() in the \Sabre\CalDAV\Schedule\Plugin::scheduleLocalDelivery method. Always passing an instance via constructor migt be an option, of via hooks or events. Overwriting the Broker would be 馃敟 so we can extend it to suit our needs. Then we can also add support for non- RFC properties such as X-RESPONSE-COMMENTs and X-NUM-GUESTS.

@szaimen

This comment was marked as off-topic.

@szaimen szaimen added needs info 0. Needs triage Pending check for reproducibility or if it fits our roadmap and removed 1. to develop Accepted and waiting to be taken care of labels Nov 26, 2022
@kesselb kesselb added 1. to develop Accepted and waiting to be taken care of and removed 1. to develop Accepted and waiting to be taken care of needs info 0. Needs triage Pending check for reproducibility or if it fits our roadmap labels Nov 27, 2022
@miaulalala miaulalala added 0. Needs triage Pending check for reproducibility or if it fits our roadmap and removed 1. to develop Accepted and waiting to be taken care of labels Nov 28, 2022
@ChristophWurst
Copy link
Member

PR is at #35761.

@ChristophWurst ChristophWurst added 3. to review Waiting for reviews and removed 0. Needs triage Pending check for reproducibility or if it fits our roadmap labels Dec 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews 21-feedback Feedback from 21.x releases 25-feedback bug feature: dav
Projects
10 participants