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

Public webdav should not be able to lock read-only public links #34347

Closed
PVince81 opened this issue Jan 31, 2019 · 6 comments · Fixed by #34351
Closed

Public webdav should not be able to lock read-only public links #34347

PVince81 opened this issue Jan 31, 2019 · 6 comments · Fixed by #34351
Assignees
Labels
feature:persistent-locking p2-high Escalation, on top of current planning, release blocker Type:Bug
Milestone

Comments

@PVince81
Copy link
Contributor

Steps

  1. Create a folder "test/share/abc"
  2. Share "share" with link with "download only" mode
  3. Copy the link token
  4. Lock "abc" on the public webdav with curl: curl -u $PUBLICTOKEN: -X LOCK http://localhost/owncloud/public.php/webdav/abc -d "<?xml version='1.0' encoding='UTF-8'?><d:lockinfo xmlns:d='DAV:'> <d:lockscope><d:exclusive/></d:lockscope></d:lockinfo>" -H 'Depth: infinite'

Expected result

Cannot lock as this is read-only access. The locks are there only for write access.

Actual result

Lock acquired.

Version

10.1.0 RC1

@davitol @individual-it @DeepDiver1975 seeing this I tend more in the direction of preventing the LOCK / UNLOCK verbs completely on public links. This will save a lot of pain and other related bugs. For now.

@PVince81 PVince81 added Type:Bug p2-high Escalation, on top of current planning, release blocker feature:persistent-locking labels Jan 31, 2019
@PVince81 PVince81 added this to the QA milestone Jan 31, 2019
@PVince81 PVince81 self-assigned this Jan 31, 2019
@PVince81
Copy link
Contributor Author

we should probably also limit this for local shares: don't allow locking on read-only shares.

@phil-davis
Copy link
Contributor

we should probably also limit this for local shares: don't allow locking on read-only shares.

however, some client with read-only access might wish to take out a lock while they are using the data in the file/folder because, for whatever reason, they want to ensure that the data does not change for the time that they hold the lock.

@ownclouders
Copy link
Contributor

GitMate.io thinks possibly related issues are #13828 (webDav locking is not working), #34268 (Hide lock token in public webdav responses), #2166 (WebDav Locking (WebDrive) not working), #34302 (public cannot unlock locks set by the public), and #21787 (Redirect public link to public Webdav URL).

@PVince81
Copy link
Contributor Author

too late... already implemented: #34349

I'm fine rejecting this if we all agree with #34347 (comment)

@phil-davis
Copy link
Contributor

I don't know what to say. It depends on the RFC, how the RFC is interpreted, how much we want to implement it, how much we want to "extend" it,...

Implementing this will not hurt anything immediately, or for the future, because it restricts the ability to use locks. So if we later want to reverse the decision and allow readers to lock read-only resources then that will extend functionality and thus will not be a nasty backward-compatible break.

In the reverse, if we release without this restriction, then it will be hard to add the restriction a year down the track.

(Similar for public links - for now we restrict what "the public" can do with locking. Then if we decide to open it up in future there is not a big backward-compatibility problem.)

@PVince81
Copy link
Contributor Author

PR to block LOCK/UNLOCK for public endpoint: #34351

might need to adjust acceptance tests as well

@lock lock bot locked as resolved and limited conversation to collaborators Feb 1, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feature:persistent-locking p2-high Escalation, on top of current planning, release blocker Type:Bug
Projects
None yet
3 participants