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

Support iMIP invitations from Mail #33001

Merged
merged 1 commit into from
Aug 23, 2022
Merged

Conversation

miaulalala
Copy link
Contributor

@miaulalala miaulalala commented Jun 23, 2022

Fixes nextcloud/mail#6803
Fixes #19144
Fixes #22532
To Do:

  • Write updated or accepted calendar events to CalDAV
  • Tests

Don't do "Fix CANCEL method for emails that are updated from UI (#33008) - the STATUS CANCELLED should also lead to an iMIP message with method CANCEL, not reply" as we're using a systemwide email address which could lead to cancellation email addresses not being processed by other mail clients.

Considerations:

  • Extend the CalendarImpl to handle these requests
  • Preconditions as specified in the RFC must be met, and the VEVENT that should be written needs to be compared to the original for both ORGANIZER and ATTENDEE
  • CANCEL requests must be from the right email address and the right organiser or they shouldn't be processed. (but what if we send them from NC from the system email? Will that work? maybe accepting the system email as a secondary email address as specified in the RFC could work.)

@miaulalala miaulalala added enhancement 2. developing Work in progress feature: caldav Related to CalDAV internals labels Jun 23, 2022
@miaulalala miaulalala self-assigned this Jun 23, 2022
Copy link

@github-advanced-security github-advanced-security bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Found 20 potential problems in the proposed changes. Check the Files changed tab for more details.

@miaulalala

This comment was marked as outdated.

@ChristophWurst

This comment was marked as outdated.

@miaulalala

This comment was marked as outdated.

@miaulalala
Copy link
Contributor Author

@st3iny sorry I forgot to tell you: there will be two new endpoints for processing the invitations/responses but you can use them almost the same as the createFromString. I just need a recipient and sender for both to do the security checks as mentioned in the RFC.

@tcitworld

This comment was marked as off-topic.

@miaulalala

This comment was marked as off-topic.

@tcitworld

This comment was marked as off-topic.

@miaulalala

This comment was marked as off-topic.

apps/dav/lib/CalDAV/CalendarImpl.php Fixed Show fixed Hide fixed
apps/dav/lib/CalDAV/CalendarImpl.php Fixed Show fixed Hide fixed
apps/dav/lib/CalDAV/CalendarImpl.php Fixed Show fixed Hide fixed
apps/dav/lib/CalDAV/CalendarImpl.php Fixed Show fixed Hide fixed
apps/dav/lib/CalDAV/CalendarImpl.php Fixed Show fixed Hide fixed
apps/dav/lib/CalDAV/CalendarImpl.php Fixed Show fixed Hide fixed
apps/dav/lib/CalDAV/CalendarImpl.php Fixed Show fixed Hide fixed
apps/dav/lib/CalDAV/CalendarImpl.php Fixed Show fixed Hide fixed
apps/dav/lib/CalDAV/CalendarImpl.php Fixed Show fixed Hide fixed
lib/private/Calendar/Manager.php Fixed Show fixed Hide fixed
lib/private/Calendar/Manager.php Fixed Show fixed Hide fixed
lib/public/Calendar/IManager.php Fixed Show fixed Hide fixed
@miaulalala
Copy link
Contributor Author

image

F*CK YEAH

apps/dav/lib/CalDAV/CalDavBackend.php Fixed Show fixed Hide fixed
apps/dav/lib/CalDAV/CalDavBackend.php Fixed Show fixed Hide fixed
apps/dav/lib/CalDAV/CalDavBackend.php Fixed Show resolved Hide resolved
apps/dav/lib/CalDAV/CalendarHome.php Fixed Show resolved Hide resolved
lib/private/Calendar/Manager.php Fixed Show resolved Hide resolved
lib/private/Calendar/Manager.php Fixed Show resolved Hide resolved
lib/private/Calendar/Manager.php Fixed Show fixed Hide fixed
lib/private/Calendar/Manager.php Fixed Show resolved Hide resolved
@miaulalala miaulalala force-pushed the enh/imip-invitations-responses branch from dd3e17a to 43f7bec Compare July 25, 2022 13:04
@miaulalala miaulalala marked this pull request as ready for review August 3, 2022 23:42
@miaulalala miaulalala requested a review from tcitworld as a code owner August 3, 2022 23:42
@ChristophWurst ChristophWurst mentioned this pull request Aug 8, 2022
21 tasks
Copy link
Member

@ChristophWurst ChristophWurst left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good so far!

apps/dav/lib/CalDAV/CalendarProvider.php Outdated Show resolved Hide resolved
apps/dav/lib/Controller/InvitationResponseController.php Outdated Show resolved Hide resolved
lib/private/Calendar/Manager.php Outdated Show resolved Hide resolved
lib/private/Calendar/Manager.php Outdated Show resolved Hide resolved
lib/public/Calendar/IManager.php Show resolved Hide resolved
lib/public/Calendar/IManager.php Show resolved Hide resolved
lib/private/Calendar/Manager.php Outdated Show resolved Hide resolved
lib/private/Calendar/Manager.php Outdated Show resolved Hide resolved
Copy link

@github-advanced-security github-advanced-security bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Found 15 potential problems in the proposed changes. Check the Files changed tab for more details.

Copy link

@github-advanced-security github-advanced-security bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Found 15 potential problems in the proposed changes. Check the Files changed tab for more details.

apps/dav/lib/CalDAV/CalendarImpl.php Fixed Show fixed Hide fixed
apps/dav/lib/CalDAV/CalendarImpl.php Fixed Show fixed Hide fixed
apps/dav/lib/CalDAV/CalendarImpl.php Fixed Show fixed Hide fixed
apps/dav/lib/CalDAV/CalendarImpl.php Fixed Show fixed Hide fixed
apps/dav/lib/CalDAV/CalendarImpl.php Fixed Show fixed Hide fixed
apps/dav/lib/CalDAV/CalendarImpl.php Fixed Show fixed Hide fixed
apps/dav/lib/CalDAV/CalendarImpl.php Fixed Show fixed Hide fixed
apps/dav/lib/CalDAV/CalendarImpl.php Fixed Show fixed Hide fixed
apps/dav/lib/CalDAV/CalendarImpl.php Fixed Show fixed Hide fixed
@kesselb
Copy link
Contributor

kesselb commented Aug 17, 2022

Thank you 👍

Looks good to me. Did a test run with 4 users.

  • Organizer Admin
  • Local guest User2
  • Local guest User3
  • External guest with Gmail account

The invitation is properly sent to all 4 guests. External guest with Gmail account accepted the invitation. Response went to Admin's inbox and IMipService updated the attendance for Admin, User2 and User 3 🥳 🥳

Updating the calendar attendance may take a while. On mailbox refresh the message is flagged as imip message and processed by a background job later. We recommend to run the background job every 5 minutes. I understand the technical reasoning yet it's not ideal.

image

image

Thunderbird shows a hint when a message contains an imip reply/cancel and if the data is already processed. I believe that would be a nice enhancement.

Run into the following "bug" while testing this PR. It seems unrelated and should not block it:

image

I accepted the invitation for User2 via Calendar. The vcard is updated properly but the imip processing does not work. Only my (the copy in my personal calendar) is updated. Organizer and other attendees don't see the updated attendance. Looked at it with Xdebug for a second but could not figure it out. The imip logic is executed however it does not emit an imip message. A starting point for further debugging could be the InviteResponseController as it works over there.

@miaulalala
Copy link
Contributor Author

I accepted the invitation for User2 via Calendar. The vcard is updated properly but the imip processing does not work. Only my (the copy in my personal calendar) is updated. Organizer and other attendees don't see the updated attendance. Looked at it with Xdebug for a second but could not figure it out. The imip logic is executed however it does not emit an imip message. A starting point for further debugging could be the InviteResponseController as it works over there.

Sorry I think I "steh auf der Leitung":

So you accepted an Invitation from Admin to User2 via Calendar, and the calendar doesn't distribute the changes to the other Attendees/ Guests?

This sounds like a bug I've seen before... @st3iny how did you implement the Accept/Decline from Calendar? Does that do a PROPPATCH?

Signed-off-by: Anna Larch <anna@nextcloud.com>
@miaulalala miaulalala force-pushed the enh/imip-invitations-responses branch from 14d5763 to 4ca4b02 Compare August 22, 2022 20:10
@st3iny
Copy link
Member

st3iny commented Aug 23, 2022

This sounds like a bug I've seen before... @st3iny how did you implement the Accept/Decline from Calendar? Does that do a PROPPATCH?

I just set the participation status of the attendee on the local copy and then save the ics again (via PUT). The rest is handled by Sabre and wasn't touched by me.

@miaulalala miaulalala merged commit 5079bf6 into master Aug 23, 2022
@miaulalala miaulalala deleted the enh/imip-invitations-responses branch August 23, 2022 07:59
@miaulalala
Copy link
Contributor Author

This sounds like a bug I've seen before... @st3iny how did you implement the Accept/Decline from Calendar? Does that do a PROPPATCH?

I just set the participation status of the attendee on the local copy and then save the ics again (via PUT). The rest is handled by Sabre and wasn't touched by me.

It was the plus sign in Daniel's test email that makes it break 🕵️

@Luncheon3462
Copy link

@ChristophWurst @miaulalala @st3iny, please help get this fixed. When Office 365 sends calendar invitations, the snappymail nextcloud app by @the-djmaze handles them perfectly. They appear in the email message and can be added to a nextcloud calendar with one-click. Maybe this code that @the-djmaze mentioned will work? https://github.com/the-djmaze/snappymail/blob/fffc04499875531bd3dd626d01baaf7a86452cd3/plugins/nextcloud/js/message.js#L111-L179

@miaulalala
Copy link
Contributor Author

@ChristophWurst @miaulalala @st3iny, please help get this fixed. When Office 365 sends calendar invitations, the snappymail nextcloud app by @the-djmaze handles them perfectly. They appear in the email message and can be added to a nextcloud calendar with one-click. Maybe this code that @the-djmaze mentioned will work? the-djmaze/snappymail@fffc044/plugins/nextcloud/js/message.js#L111-L179

Hi Luncheon, please stop mass tagging and commenting on old tickets and PRs. You're creating a lot of noise, not just for us but also the-djmaze. That isn't cool.

If you have a feature request or bug to report, use the new issue button in the repository AFTER you've searched if an issue already exists. If it does, please refrain from commenting unless you have logs / debugging steps to add, and add your reaction to the ticket instead. If you're a developer, you can also contribute your own PRs and we will work with you to get them merged.

Cheers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4. to release Ready to be released and/or waiting for tests to finish enhancement feature: caldav Related to CalDAV internals
Projects
None yet
6 participants