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

[CalDAV] PUT on CalDAV collection should return 403 instead of 404 #2355

Open
rfc2822 opened this issue Oct 5, 2020 · 3 comments
Open

[CalDAV] PUT on CalDAV collection should return 403 instead of 404 #2355

rfc2822 opened this issue Oct 5, 2020 · 3 comments

Comments

@rfc2822
Copy link

rfc2822 commented Oct 5, 2020

Hi,

As long as there's no write access to Deck's CalDAV collections, the server restricts write access as expected. However, it returns 404 instead of 403.

Steps to reproduce:

  1. Upload (PUT) a task (VTODO) to a Deck CalDAV collection.

Actual result:
Currently, it returns 404 Not found when you try to upload a task.

Expected result:
Deck should return 403 Forbidden:

https://tools.ietf.org/html/rfc3744#section-3
Servers MUST report a 403 "Forbidden" error if access is denied, except in the case where the privilege restricts the ability to know the resource exists, in which case 404 "Not Found" may be returned.

Because the client has read access, it knows which resources exist and thus there's no need for 404.

The response body should contain:

https://tools.ietf.org/html/rfc3744#section-7.1.1
If an HTTP method fails due to insufficient privileges, the response body to the "403 Forbidden" error MUST contain the DAV:error element, which in turn contains the <DAV:need-privileges> element, which contains one or more <DAV:resource> elements indicating which resource had insufficient privileges, and what the lacking privileges were: […]

With 403 and <DAV:need-privileges>, clients can show a meaningful error message.

Tested with:
Nextcloud 20, Deck 1.1.0

@juliushaertl
Copy link
Member

juliushaertl commented Oct 5, 2020

Yes, definitely makes sense as discussed.

We actually already throw a Forbidden exception in the createFile method of our calendar implementation, but maybe there is some additional check on the node existence upfront somewhere in the server dav code. I'll look into that.

@juliushaertl juliushaertl added this to the 1.1.1 milestone Oct 5, 2020
@juliushaertl juliushaertl self-assigned this Oct 5, 2020
@juliushaertl juliushaertl modified the milestones: 1.1.1, 1.1.2, 1.1.3 Oct 13, 2020
@juliushaertl juliushaertl modified the milestones: 1.1.3, 1.2.0, 1.2.1 Nov 11, 2020
@juliushaertl juliushaertl modified the milestones: 1.2.1, 1.2.2, 1.2.3 Nov 18, 2020
@juliushaertl juliushaertl modified the milestones: 1.2.3, 1.2.4 Jan 4, 2021
@juliushaertl juliushaertl modified the milestones: 1.2.4, 1.3.3, 1.4.1, 2.0.0 Apr 7, 2021
@Redsandro
Copy link

Redsandro commented May 8, 2021

I would like to point out that this still affects thousands of /e/os and custom DAVx⁵ installations across the world.

With a 404, some clients keep trying to PUT. At least with a 403, the client knows to give up. I know this issue is probably superseded by #2399, but if you can find a relatively simple way to change that 404 to 403 (it's probably more complex than it looks), please to that in the meantime.

In Deck's defense, the CalDAV implementation came a long way. :)

@juliushaertl juliushaertl modified the milestones: 1.5.0, 1.6.0 Nov 5, 2021
@jurgenhaas
Copy link

As I also reported in the DAVx5 forum it's causing the same 404 error but changing the label of a card on the Android device still gets synced back to Nextcloud. Feels like a partial sync?

@juliushaertl juliushaertl modified the milestones: 1.6.0, 1.7.0 Dec 30, 2021
@juliushaertl juliushaertl removed this from the 1.7.0 (Nextcloud 24) milestone Apr 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants