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

apps/dav: add some OSX specific quirks. #36800

Merged
merged 1 commit into from
Mar 13, 2023

Conversation

rotdrop
Copy link
Contributor

@rotdrop rotdrop commented Feb 21, 2023

Summary

Following @georgehrke suggestion in the linked issue this PR adds a special QuirksPlugin which tweaks report requests from the half-eaten-apple OS to include the apply-to-principal-collection-set flag.

This is done by a Sabre ServerPlugin with beforeMethod:* listener which decodes the HTTP_USER_AGENT and a report listener which then tweaks a {DAV:}principal-property-search report request to have the flag set.

TODO

  • implement tests

Checklist

@rotdrop
Copy link
Contributor Author

rotdrop commented Feb 21, 2023

Actually, the delegation dialog and the participants dialog of the OSX calendar app in "Ventura" now finds the principals. Inviting participants seems to work, delegation is still flaky: the delegation is transmitted to the NC server but the OSX calendar app somehow has problems with the response. The web-server access log however only shows

REPORT /remote.php/dav/principals/users/claus/calendar-proxy-write/ HTTP/1.1" 207 1203

when updating the delegation settings.

@rotdrop rotdrop force-pushed the feature/dav-macos-quirks branch from 22b6c01 to b1156ce Compare February 21, 2023 20:42
@rotdrop
Copy link
Contributor Author

rotdrop commented Feb 21, 2023

This is done by a Sabre ServerPlugin with beforeMethod:* listener which decodes the HTTP_USER_AGENT and a report listener which then tweaks a {DAV:}principal-property-search report request to have the flag set.

What can be used in place of the asterisk? Parsing the user-agent with a regexp is probably a little bit costly, and here only necessary for the report requests.

@rotdrop rotdrop force-pushed the feature/dav-macos-quirks branch from 6543b7b to 11cc75a Compare February 21, 2023 20:57
@juliusknorr juliusknorr added 3. to review Waiting for reviews feature: dav feature: caldav Related to CalDAV internals labels Feb 21, 2023
@szaimen szaimen added this to the Nextcloud 26 milestone Feb 22, 2023
@rotdrop rotdrop force-pushed the feature/dav-macos-quirks branch 2 times, most recently from 2b24c42 to 4671fc6 Compare February 23, 2023 08:01
@szaimen szaimen requested a review from tcitworld February 23, 2023 08:18
apps/dav/lib/Connector/Sabre/QuirksPlugin.php Outdated Show resolved Hide resolved
@skjnldsv skjnldsv mentioned this pull request Feb 23, 2023
@rotdrop rotdrop force-pushed the feature/dav-macos-quirks branch from 4671fc6 to a089db9 Compare February 24, 2023 10:51
@miaulalala miaulalala self-requested a review February 24, 2023 10:53
@rotdrop rotdrop force-pushed the feature/dav-macos-quirks branch from 4b2e884 to 345751f Compare February 24, 2023 10:55
Copy link
Member

@tcitworld tcitworld left a comment

Choose a reason for hiding this comment

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

Thanks, looks great!

@rotdrop rotdrop force-pushed the feature/dav-macos-quirks branch 2 times, most recently from 43ada45 to 57c5c15 Compare February 27, 2023 13:39
@tcitworld
Copy link
Member

Do you mind reverting your composer-related changes other than adding the new class in apps/dav/composer/composer/autoload_static.php and the second part of apps/dav/composer/composer/autoload_classmap.php? You likely have a different composer version installed that cause the rest of the changes.

Please squash your commits as well.

@rotdrop
Copy link
Contributor Author

rotdrop commented Feb 28, 2023

Do you mind reverting your composer-related changes other than adding the new class in apps/dav/composer/composer/autoload_static.php and the second part of apps/dav/composer/composer/autoload_classmap.php? You likely have a different composer version installed that cause the rest of the changes.

I have probably just a recent composer version installed, which no longer support PHP 5. But for the question: I do not mind at all. However, ideally the Nextcloud core developers would just regenerate the autoload-files with whatever composer version they usually use. I think without the composer changes at least the continuous integration test has no chance to succeed.

Please squash your commits as well.

I do not mind, but isn't this done automatically during merge? Doesn't matter, I'll squash them.

@rotdrop
Copy link
Contributor Author

rotdrop commented Feb 28, 2023

Please squash your commits as well.

I do not mind, but isn't this done automatically during merge? Doesn't matter, I'll squash them.

Actually, I would suggest to squash all commits with the exception of the composer changes.

@rotdrop
Copy link
Contributor Author

rotdrop commented Feb 28, 2023

Please squash your commits as well.

I do not mind, but isn't this done automatically during merge? Doesn't matter, I'll squash them.

Actually, I would suggest to squash all commits with the exception of the composer changes.

@tcitworld In order to avoid unnecessary work I'll wait for a response from your side. Of course, it is not a problem to just squash all commits in to one, I just think that it makes sense to keep all generated files separate from the real changes.

@tcitworld
Copy link
Member

However, ideally the Nextcloud core developers would just regenerate the autoload-files with whatever composer version they usually use.

Yeah, I guess this is done one in a while, but let's not create conflits if we can avoid them.

I do not mind, but isn't this done automatically during merge?

This is not enabled for this repository, not sure why, probably Github drops Sign-off information during squashing.

@rotdrop rotdrop force-pushed the feature/dav-macos-quirks branch 2 times, most recently from e7f443a to ced9c4c Compare March 7, 2023 11:40
Signed-off-by: Claus-Justus Heine <himself@claus-justus-heine.de>
@rotdrop rotdrop force-pushed the feature/dav-macos-quirks branch from ced9c4c to 64e9dcc Compare March 7, 2023 15:24
@rotdrop
Copy link
Contributor Author

rotdrop commented Mar 7, 2023

Do you mind reverting your composer-related changes other than adding the new class in apps/dav/composer/composer/autoload_static.php and the second part of apps/dav/composer/composer/autoload_classmap.php? You likely have a different composer version installed that cause the rest of the changes.

Please squash your commits as well.

I have just also squashed the composer changes into the remaining single commit.

@blizzz blizzz mentioned this pull request Mar 7, 2023
@blizzz blizzz modified the milestones: Nextcloud 26, Nextcloud 27 Mar 7, 2023
@miaulalala miaulalala merged commit ac90fa2 into nextcloud:master Mar 13, 2023
@labor4
Copy link
Contributor

labor4 commented Mar 22, 2023

Are there intentions to backport to NC 26 ?

@z3ky
Copy link

z3ky commented Nov 28, 2023

Does this plugin still works? If yes, how do I activate it?

@JohnPritchard
Copy link

I am brand spanking new to NextCloud, I have just completed installation for the first time. I'm running Nextcloud Hub 9 (30.0.1) -- which I assume means I'm running NextCloud 30.0.1.
Is this "plugin" still relevant? How do I install/enable it?
Many thanks in advance for any help and/or advice.

@labor4
Copy link
Contributor

labor4 commented Nov 6, 2024

I am brand spanking new to NextCloud, I have just completed installation for the first time. I'm running Nextcloud Hub 9 (30.0.1) -- which I assume means I'm running NextCloud 30.0.1. Is this "plugin" still relevant? How do I install/enable it? Many thanks in advance for any help and/or advice.

This is not a plugin but a code change. It has been implemented. DAV is core functionality. In this case a fix for the "Calendar" app.

@JohnPritchard
Copy link

That's great news!
Indeed I can connect to my NextCloud Calendar Server from macOS (15 == Sequoia) and iOS (18). I see events created via the webUI in macOS and iOS and vice versa.
But unfortunately I am still having trouble setting up delegates, can I/should I report the issues here? Or there somewhere else I should make an Issue/bug report/...

@labor4
Copy link
Contributor

labor4 commented Nov 7, 2024

That's great news! Indeed I can connect to my NextCloud Calendar Server from macOS (15 == Sequoia) and iOS (18). I see events created via the webUI in macOS and iOS and vice versa. But unfortunately I am still having trouble setting up delegates, can I/should I report the issues here? Or there somewhere else I should make an Issue/bug report/...

This issue is closed and outdated.

When you think there is still a problem, create a new bug report/issue.

btw. in the meantime: apple is notorious for bending even their own standards. you can delegate via the nextcloud web interface, which will reflect back to apple calendar app. but your contribution is helpfull, so when you describe the symptoms/observations closely and post a report, thats the right way to confront this problem.

another btw: if you are going to deal with this specific issue, if you can, its a good idea to include the webservers access.log entries in the report, that apple calender triggers during your attempt.

@JohnPritchard
Copy link

Hi labor4,
Thanks for the clear instructions...
I can easily appreciate that it is caused by Apple bending... it is also entirely possible I've not got everything quite right, I just spent several hours getting my half-working SSL certificate verification working 100% -- I had to provide the SSLCACertificateFile of my certificate provider, which I hadn't realised.

In the meantime, it would be great if I can set up delegatation via the nextcloud web interface, but I've not been able to find that, would you be so kind as to point me in the right direction?

In any case, many thanks for your help and assistance.
J

@rotdrop
Copy link
Contributor Author

rotdrop commented Nov 7, 2024

Hi labor4, Thanks for the clear instructions... I can easily appreciate that it is caused by Apple bending... it is also entirely possible I've not got everything quite right, I just spent several hours getting my half-working SSL certificate verification working 100% -- I had to provide the SSLCACertificateFile of my certificate provider, which I hadn't realised.

In the meantime, it would be great if I can set up delegatation via the nextcloud web interface, but I've not been able to find that, would you be so kind as to point me in the right direction?

In any case, many thanks for your help and assistance. J

Nope, AFAIK this is still not exposed in the web interface, though it should be supported by the calendar backend. See here:

nextcloud/calendar#2706

@labor4
Copy link
Contributor

labor4 commented Nov 7, 2024

thanks

given said issue (still valid?), we maybe speak of different technical things.

i tested it a while ago and „shared“ a cal to another user in web, and found it to be handed down to apple calender, seemingly in a way that was transparent (as in „does not know“) to the client. how this really differentiates against a proper „caldav delegation“ i do not know.

i did have the (unproven) suspicion that this may be a „dontworry“ replacement method for this need that works better. same way NC finally ditched the apple-specific calendar-invitation chaos and went the more cross/stable google-way with mails and links. but thats just my guts speaking. 🙏😁

carefull testing for usability by the user is advised, i guess.

sorry. dont know more.

is there an arguement that „sharing a calendar in some way or meaning internally via web to another user“ is not possible?

@rotdrop
Copy link
Contributor Author

rotdrop commented Nov 7, 2024

thanks

given said issue (still valid?), we maybe speak of different technical things.

i tested it a while ago and „shared“ a cal to another user in web, and found it to be handed down to apple calender, seemingly in a way that was transparent (as in „does not know“) to the client. how this really differentiates against a proper „caldav delegation“ i do not know.

i did have the (unproven) suspicion that this may be a „dontworry“ replacement method for this need that works better. same way NC finally ditched the apple-specific calendar-invitation chaos and went the more cross/stable google-way with mails and links. but thats just my guts speaking. 🙏😁

carefull testing for usability by the user is advised, i guess.

sorry. dont know more.

"delegation" means that you let someone else edit your calendars on behalf of yourself, you remain the organizer. It is not visible that it wasn't you who edited things. The use case is e.g. when "the big boss" delegates his calendars to a clerk who manages his appointments.

This is different from a shared writable calendar, here the person who creates an event is the organizer which then in general is different from the owner of the calendar.

And yes: if you share a calendar to someone else then this calendar will appear in that other ones caldav collection and is visible through the caldav protocol.

And yes: the issue a mentioned in my previous post is still valid, it is still not possible to manage calendar delegation in the frontend.

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 feature: caldav Related to CalDAV internals feature: dav
Projects
None yet
Development

Successfully merging this pull request may close these issues.

macOS calendar: Can't search for delegates to add
9 participants