Send invitations for shared calendars

kesselb commented 3 years ago

Ref: https://github.com/nextcloud/server/pull/15676

It's not possible (anymore) to send invitations for an event for shared calendars. This behavior is confusing for end users.

Allowing sharees to send invitation in shared calendars was a mistake back then, there are many unresolved topics, especially if the sharee is also an attendee of the event:

when editing, does the sharee edit for themselves or on behalf of the calendar owner?

when deleting, is the sharee declining the event for themselves or cancelling it for everyone?

...

The CalDAV standard does not have answers to these problems yet. Until then, we should not allow sharees to send invitations in shared calendars.

https://sabre.io/dav/caldav-sharing/ mentions

Unfortunately there is no final RFC how shared calendars should work. Yet the drafts could be a starting point. draft-pot-caldav-sharing-00 describes those open issues:

DAV:owner requirement for scheduling. I think this is problematic...
I don't think we should allow sharees that have access to an invite for which they are the attendee for, via the organizers shared calendar, to allow them to make attendee-related changes. The entire collection should operate as if the operation is on behalf of the organizer.

We don't implement https://github.com/sabre-io/dav/blob/master/lib/CalDAV/Backend/SharingSupport.php interface yet. That might be something to start with. There is already some logic connected to the above interface in sabre that might be useful.

The example implementation https://github.com/sabre-io/dav/blob/c1afdc77a95efea6ee40c03c45f57c3c0c80ec22/lib/CalDAV/Backend/PDO.php#L1308-L1465 uses a database table calendarinstances that looks similar to oc_calendars and oc_dav_shares. For example calendarcolor is something we store with the calendar. But for a shared calendar the color is something a user may change. So this needs to go to oc_dav_shares (calendarinstance in the sabre example).

ChristophWurst commented 3 years ago

Some related sharing limitations are discussed at https://github.com/nextcloud/server/issues/5050.

favincen commented 3 years ago

We don't implement https://github.com/sabre-io/dav/blob/master/lib/CalDAV/Backend/SharingSupport.php interface yet. That might be something to start with.

👍 ! Seems pretty close to the suggestion I made in https://github.com/nextcloud/server/pull/15676#issuecomment-692860743, regarding the issue that currently used "getUsersOwnCalendars" does not list calendars shared to the user, and that some kind of "getUserEditableCalendars" method would be of great help. 😇

rasos commented 2 years ago

Workaround: Create a team meeting in your personal calendar, check sending e-mail notifications, and then change the assignment of the event to your team calendar. Drawback: if you change the date, nobody is notified again, unless you assign the event back to a calendar you own, do the shift and assign it back to the team calendar.

As this is a bit hard to explain and understand we think that an extra method "getUserEditableCalendars" that controls the Send email checkbox would be the experience that users expect. Now you see this checkbox even if you don't have the rights to send an e-mail (and you wonder why nobody confirms the event).

We always recommend that teams should create an extra user who then owns the team's root folder, addressbook and also calendar. Those should not belong to a person, as the team would loose those assets if a founder leaves the team.

xeruf commented 2 years ago

Please, such a basic feature has been pushed of for over 4 years? These are the kinda qualms that make the life of open-source enthusiasts like me hard, as people will not use these tools when such basics silently fail! It took me hours of debugging until I stumbled here.

ChristophWurst commented 2 years ago

It's on our list of planned tasks but thanks for the kind words :+1:

xeruf commented 2 years ago

Let me add some actual kind words :) I have been wanting to setup Nextcloud for years, and finally got the chance in our open-source-friendly startup. I am in love with the diversity it offers and customizability, everyone is ready to adopt it - were it not for unpolished corners like these, that lead them to continue with Google (even though they themselves don't want to!) until they are resolved. I so badly want Nextcloud to succeed and build a product together with you to displace Teams, GSuite and all their crap, even Github once Gitea federation is implemented, all with a SSO through a suite like Stackspin. But before all that, we need to develop a rock-solid foundation that convinces non-techy business users, too.

rotdrop commented 1 year ago

Isn't this somehow also related to nextcloud/calendar#2706? At least a group-shared calendar could be owned by a bot which then delegates access to the group members which could act on behalf of the bot. There are rumours that delegation already works in the backend ...

ChristophWurst commented 1 year ago

Delegation is for accounts. Sharing is for individual calendars: https://sabre.io/dav/caldav-proxy/

There are rumours that delegation already works in the backend ...

Source? I checked that recently and we don't have the db tables required for Sabre to set up delegation.

rotdrop commented 1 year ago

Delegation is for accounts.

Sorry, yes you are right. This is a different issue.

Source? I checked that recently and we don't have the db tables required for Sabre to set up delegation.

Well, I am referring to https://github.com/nextcloud/calendar/issues/2706#issuecomment-729489165 which in turn refers to the pull-request #16666.

rotdrop commented 1 year ago

For the record: I did some brief code-grepping and came up with a hacky pull request.

The pull-request is based on the original (reverted) commit but makes sure that the Sabre-library can understand the responses sent back by the invitees. Technical details and not least the code can be found in the pull-request #36756. For short: this is more or less the thing proposed in https://github.com/nextcloud/server/pull/15676#issuecomment-692860743.

There are of course loose ends. This is just not "the solution" but a mere code-study.

rotdrop commented 1 year ago

responses sent back by the invitees. Technical details and not least the code can be found in the pull-request #36756. For short: this is more or less the thing proposed in #15676 (comment).

Most tests seem to pass for the moment. The PR also implements that SharingSupport interface. In principle this would enable the frontend app -- which still claims that invitations have been sent although this does not happen -- to distinguish shared calendars from non-shared calendars.

The PR enables invitations and responses for shared writable calendars.

if the invitees have no access or only read access to the shared calendar then they will receive a copy of the event in their own default calendars when accepting the invitation
invitees having also write access to the shared calendar ATM will not receive a copy of the event

This implies that changing the sharing ACLs (unshare, share with already accepted attendees, change between read-only and read-write) of shared calendars is problematic.

rotdrop commented 1 year ago

As far as I understand this affair, in the current state of the underlying Sabre DAV library it is impossible to send invitations for events in shared calendars without doing "nasty" stuff as is done in my draft pull-request #36756.

Sending invitations

The first point -- which was already addressed by the original, now reverted patch -- is that Sabre in its scheduler plugin uses the calendar owner to determine the organizer principal:

https://github.com/sabre-io/dav/blob/2d8f6d9b9851a3d5fec007b7033d86b1dc241663/lib/CalDAV/Schedule/Plugin.php#L326-L328

Later the email-addresses obtained there are used in order to distinguish the organizer from mere invited participants, here $userHref is more-or-less the list of email-addresses of the organizer:

https://github.com/sabre-io/vobject/blob/198196c9cfe436c8c0ee5637a5129a455c9bf404/lib/ITip/Broker.php#L249-L251

and

https://github.com/sabre-io/vobject/blob/198196c9cfe436c8c0ee5637a5129a455c9bf404/lib/ITip/Broker.php#L437-L445

So in order to have notifications be sent to the organizer of an event in a shared calendar the scheduler plugin needs to be changed. From a point of view of having "clean source code" this is bad as we have to override a method which probably was not meant to be re-coded entirely.

If we do so then notifications are already sent to participants.

Receiving invitation responses

Now comes the next point: when the participants answer the invitations the results are finally processed here:

https://github.com/sabre-io/dav/blob/2d8f6d9b9851a3d5fec007b7033d86b1dc241663/lib/CalDAV/Schedule/Plugin.php#L492

Here the back-end method getCalendarObjectByUID() is used and that one is documented to return only calendar objects owned by the respective principal. Hence the responses of the attendees never make there way into the calendar object in the shared calendar.

Role of the `SharingSupport` interface

I do not have the impression that this brings as any closer to sending invitations for events in shard calendars. I think this SharingSupport only defines methods in order the define calendar sharing? In order to implement this we would just have to wrap the existing Nextcloud functionality into this interface. This is done experimentally in this PR: #36766.

But this does not help with the invitations.

Frontend bug

The Nextcloud calendar app has the bug that it claims that notifications have been sent: nextcloud/calendar#4983

Workarounds like calendar delegation or resource calendars

delegation is not the same but would hack some scenarios. However, there is no frontend support, see nextcloud/calendar#2706 and the interplay with calendar apps which in principle could define delegations is at least in the case of the OSX calendar app broken (here by bugs in that half-eaten-apple app, see #17053)
one could use resource calendars for some things but then one would have to remember to always add the resource to the attendees such that it is invited.

Next steps

Well, I think I will remove the "draft" attribute from my pull request #36756. The situation is not ideal, but neither is the current situation ideal which consists of a missing backend feature (this issue) and a bug in the frontend (this issue: nextcloud/calendar#4983) . I would really like to have this functionality.

ChristophWurst commented 1 year ago

From a point of view of having "clean source code" this is bad as we have to override a method which probably was not meant to be re-coded entirely.

What this, or the solution in general, make sense to be done in sabre itself?

rotdrop commented 1 year ago

From a point of view of having "clean source code" this is bad as we have to override a method which probably was not meant to be re-coded entirely.

What this, or the solution in general, make sense to be done in sabre itself?

It cannot be entirely be done in Sabre as the getCalendarObjectByUID() has to be implemented in the backend implementation. However, it would "feel better" if in Sabre the comment could be changed to "allow" returning objects also from calendars the principal has write access to.

The other change -- not using the owner of the calendar but the organizer in the scheduler plugin -- would make more sense in Sabre. As Sabre is used in more than one project there will probably be compatibility concerns.

At any rate it would probably good to open an issue in the sabre-io project. This will not make things happen faster.

ChristophWurst commented 1 year ago

At any rate it would probably good to open an issue in the sabre-io project. This will not make things happen faster.

It could give a better sense of the compatibility concerns.

I do not mean to cause more work or effort than really required. I'm just thinking that it could be even more efficient to make changes in Sabre itself if that requires fewer hacks in the end.

rotdrop commented 1 year ago

@ChristophWurst "It would": I totally agree. I just fear that I might at some point just implement it in my own NC instances and leave the community behind, just for lack of time. Anyhow, of course it is a very good idea -- or better: the name of the game -- to rather try to fix things upstream than to hack around downstream.

Just now I have posted to sabre-io: sabre-io/dav#1454 and sabre-io/vobject#614

My texts there are a little bit sparse. We will see what happens.

Mer0me commented 1 year ago

The only workaround we found to resolve this issue at work is to use Thunderbird with client-side scheduling. It's not perfect but we can send invitations from owned or shared calendar.

Limits of this solution :

The server is not up to date if the user doesn't refresh event when he receives an answer
If a user accept from the Nextcloud Mail App, the answer is never received by the organizer
When there is multiple attendees, only the organizer knows who have accepted/refused invitation, other attendees can only see their own status
In some case, when you accept too quickly an invitation just received, TB is not able to update the calendar
If a user invite from the Calendar App, attendees can accept with TB interface or click on the button in the mail body resulting in different behaviour
needs to deploy TB everywhere...

When moving from (even very old - 2010) Outlook/Exchange system, users can be disappointed to loose some functionalities. It should be a top priority to resolve this issue in order to compete with O365 and GSuite in small/medium business context.

rotdrop commented 1 year ago

@Mer0me Do you see a chance to test my PR #36756?

Mer0me commented 1 year ago

@rotdrop Tests are very difficult in a near-production environment with 400+ users. But I follow your PR since few weeks, I will post relevant information about it if I can.

aleld commented 1 year ago

The only workaround we found to resolve this issue at work is to use Thunderbird with client-side scheduling. It's not perfect but we can send invitations from owned or shared calendar.

Limits of this solution :
* The server is not up to date if the user doesn't refresh event when he receives an answer

* If a user accept from the Nextcloud Mail App, the answer is never received by the organizer

* When there is multiple attendees, only the organizer knows who have accepted/refused invitation, other attendees can only see their own status

* In some case, when you accept too quickly an invitation just received, TB is not able to update the calendar

* If a user invite from the Calendar App, attendees can accept with TB interface or click on the button in the mail body resulting in different behaviour

* needs to deploy TB everywhere...
When moving from (even very old - 2010) Outlook/Exchange system, users can be disappointed to loose some functionalities. It should be a top priority to resolve this issue in order to compete with O365 and GSuite in small/medium business context.

This^

I am in the same position trying to get this work for a small business. If this will not be fixed within this year I can't hold on to nc anymore and am forced to move to O365 as much as I want to avoid that :( I appreciate all of the amazing work done, but its barely not feasible in an work environment right now.

ChristophWurst commented 5 months ago

We have found the Sabre invitation plugin to skip invites if the organizer of the event is not the owner of the calendar. Right now that happens for shared calendars because \OCA\DAV\CalDAV\Calendar::getOwner overwrites \Sabre\CalDAV\Calendar::getOwner. Upstream returns the principaluri of the owner, we return the principaluri of the sharee. We want to test if this revives the invites in shared calendars. We might need some adjustments for other places that use the getOwner method.

The method overwrite was added in https://github.com/owncloud/core/pull/21964, before Sabre supported sharing

subroutineox commented 2 months ago

i really wish you guys all the best in fixing this issue - it is really the main problem for us to fully commit to nextcloud Hub. thanks for your hard work!

SebastianKrupinski commented 2 months ago

@subroutineox are you interested in testing the fix? and giving us feed back?

Here is the fix.

https://github.com/nextcloud/server/pull/45054

subroutineox commented 2 months ago

@subroutineox are you interested in testing the fix? and giving us feed back?

Here is the fix.

45054

Thank you for the offer. Sadly i dont have a testing environment and might not have the knowledge that might be needed to properly help. Best wishes

bpmartin20 commented 2 months ago

I'd be very pleased to help test the fix, however I'm not wild about moving my whole stable 29.0.4 installation to a development version. Is there a way I can install just this patch, or is it likely to have dependencies that aren't present in stable?

outofcontrol commented 2 months ago

We might be able to test. Need a couple of days to ensure I have the allowance from my client that is most affected by this, to snaphot/upgrade/test their install. Will know more in a couple of days.

SebastianKrupinski commented 2 months ago

@outofcontrol just an FYI, the fix is two parts. One part is to fix the issue with shared calendars in the server code, that part is a straight copy and paste of the affected file. The second part is to enable the option in the calendar, this would need to be compiled with npm first (npm ci && npm run build) as it modifies the some of the java script in the UI.

bpmartin20 commented 2 months ago

So in the repository I see two branches for 26668:

origin/feat/26668/notifications-for-shared-calendars-2
origin/fix/issue-26668 They appear to conflict, as they have updates for some of the same files. Which should I be testing?

SebastianKrupinski commented 2 months ago

@bpmartin20 you should test this one. https://github.com/nextcloud/server/pull/45054

Please read the instructions there is also a patch for the calendar app that needs to be pulled

nextcloud / server