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

fix(settings): make trailing slash for caldav/carddav redirects optional #46079

Merged
merged 1 commit into from
Jun 25, 2024

Conversation

MichaIng
Copy link
Member

@MichaIng MichaIng commented Jun 24, 2024

Summary

#43939 moved the CalDAV/CardDAV redirect checks from the frontend to a new backend API.

Since the backend does not send an authentication header, checking for the expected response code 207 of the DAV endpoint does not work anymore. Hence the URL of the last redirect is checked instead.

This URL is expected to contain a trailing slash, which was not required before, since the DAV endpoint works properly without it (when authenticated).

While a trailing slash in the redirect does no harm, it causes many setups to throw an admin panel warning, while in fact the redirects work properly. Furthermore, the proposed "/.well-known/carddav" => "/remote.php/dav/" redirect leads to double slashes, when doing a request to "/.well-known/carddav/", which seems more wrong than right.

This change makes the trailing slash optional, hence old and adjusted setups won't throw the warning anymore, and the DAV endpoint works well in both cases.

The motivation behind the change is to prevent admins from doing practically unnecessary webserver config changes, after upgrading to Nextcloud 29, to mute the too strict setup warning.

TODO

  • While documentation changes which added the trailing slash could be reverted, it is not required. It works either way.

Checklist

#43939 moved the CalDAV/CardDAV redirect checks from the frontend to a new backend API.

Since the backend does not send an authentication header, checking for the expected response code 207 of the DAV endpoint does not work anymore, hence the URL of the last redirect is checked instead.

This URL is expected to contain a trailing slash, which was not required before, since the DAV endpoint works properly without it (when authenticated).

While a trailing slash in the redirect does no harm, it causes many setups to throw an admin panel warning, while in fact the redirects work properly. Furthermore, the proposed "/.well-known/carddav" => "/remote.php/dav/" redirect leads to double slashes, when doing a request to "/.well-known/carddav/", which seems more wrong then right.

This change makes the trailing slash optional, hence old and adjusted setups won't throw the warning anymore, and the DAV endpoint works well in both cases.

Signed-off-by: MichaIng <micha@dietpi.com>
Copy link
Contributor

@come-nc come-nc left a comment

Choose a reason for hiding this comment

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

👍

@come-nc
Copy link
Contributor

come-nc commented Jun 25, 2024

/backport to stable29

@come-nc come-nc merged commit 9496ce6 into master Jun 25, 2024
165 checks passed
@come-nc come-nc deleted the fix/well-known-checks branch June 25, 2024 08:24
Copy link
Contributor

github-actions bot commented Jul 9, 2024

Hello there,
Thank you so much for taking the time and effort to create a pull request to our Nextcloud project.

We hope that the review process is going smooth and is helpful for you. We want to ensure your pull request is reviewed to your satisfaction. If you have a moment, our community management team would very much appreciate your feedback on your experience with this PR review process.

Your feedback is valuable to us as we continuously strive to improve our community developer experience. Please take a moment to complete our short survey by clicking on the following link: https://cloud.nextcloud.com/apps/forms/s/i9Ago4EQRZ7TWxjfmeEpPkf6

Thank you for contributing to Nextcloud and we hope to hear from you soon!

(If you believe you should not receive this message, you can add yourself to the blocklist.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: NC29 .well-known URLs, failed on: /.well-known/caldav
4 participants