fix(dav): return 403 not 404 on denied write to a readable resource#61695
Open
ndo84bw wants to merge 1 commit into
Open
fix(dav): return 403 not 404 on denied write to a readable resource#61695ndo84bw wants to merge 1 commit into
ndo84bw wants to merge 1 commit into
Conversation
Assisted-by: ClaudeCode:claude-opus-4-8[1m] Signed-off-by: Nico Donath <ndo84bw@gmx.de>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Write access to a shared read-only calendar or shared read-only address book is answered by the server with
404 Not Foundinstead of403 Forbidden. The user can see the resource (READ is allowed, GET returns 200 and it appears in PROPFIND), but a PUT or DELETE on it reports404. Clients cannot derive a meaningful error from that.The cause is
DavAclPlugin::checkPrivileges: on an ACL denial,404is thrown unconditionally for every non-owner (protection against resource enumeration, originally owncloud/core#22578).RFC 3744 (WebDAV Access Control Protocol) Section 3:
So returning 404 is fine, but only if the user cannot read the resource. Since the calendar is shared read-only, the user does have the right to see it and should get a
403.The fix adds a read check before the
404: if the requesting user hasREADon the resource, the real authorization error is returned - a403with theDAV:need-privilegesbody (RFC 3744 Section 7.1.1) - instead of the404. Without read permission it stays404. (An owner already received a plain403 Forbiddenfrom the pre-existing owner check and is unchanged.)The change sits at a single central point through which all DAV ACL checks pass; it therefore applies uniformly to CalDAV and CardDAV and to all write operations (PUT, PROPPATCH, MKCOL/MKCALENDAR, bind/unbind = DELETE/MOVE/COPY).
Side note: the CalDAVTester suite
apps/dav/tests/testsuits/caldavtest/tests/CalDAV/sharing-calendars.xml(test suite "Change to read-only calendar"), commented out since 2016, already expects403for a read-only PUT - the current behavior deviates from that. Reactivating this suite would be a reasonable follow-up step, but it is not part of this PR.Tested
Unit
Three scenarios, each using a DataProvider against a
CalendarAND anAddressBooknode.NeedPrivileges(403)NotFound(404) with type-correct messageForbidden(403) << (Example is Birthday Calendar)Live (on NC master)
Calendar and address book shared read-only by User1 to User2 via the web interface. User2 then attempted writing events into the read-only shared calendar and writing contacts into the read-only shared address book.
Via Thunderbird and via curl. Result:
404403404403404403404403207207(unchanged)404404(enumeration protection unchanged)With Curl only:
404403404403404404(enumeration protection unchanged)405405(Apache error, not our code)Manual reproduction (curl)
Setup:
User1shares their "personal" calendar read-only withUser2.User2mounts it at.../calendars/User2/personal_shared_by_User1/.Checklist
3. to review, feature component)stable32)AI (if applicable)
sharing-calendars.xml