nextcloud / deck

🗂 Kanban-style project & personal management tool for Nextcloud, similar to Trello
https://apps.nextcloud.com/apps/deck
GNU Affero General Public License v3.0
1.2k stars 273 forks source link

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

Open rfc2822 opened 3 years ago

rfc2822 commented 3 years ago

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 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 commented 3 years ago

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.

Redsandro commented 3 years ago

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. :)

jurgenhaas commented 2 years ago

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?