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

Persistent lock: cannot unshare from self #33847

Closed
PVince81 opened this issue Dec 10, 2018 · 8 comments · Fixed by #34099
Closed

Persistent lock: cannot unshare from self #33847

PVince81 opened this issue Dec 10, 2018 · 8 comments · Fixed by #34099

Comments

@PVince81
Copy link
Contributor

Steps

  1. Create two users "user1" and "user2"
  2. Login as "user1"
  3. Create a folder "test"
  4. Share "test" with "user2"
  5. Lock "test" with curl -u user1:test -X LOCK http://localhost/owncloud/remote.php/webdav/test -d "<?xml version='1.0' encoding='UTF-8'?><d:lockinfo xmlns:d='DAV:'> <d:lockscope><d:shared/></d:lockscope></d:lockinfo>"
  6. Login as "user2"
  7. Unshare from self the folder "test".

Expected result

Unshare works as it doesn't affect the original folder.

Actual result

Unshare fails.

{"reqId":"P1p5eOUD9iBxKNIO0ITj","level":4,"time":"2018-12-10T16:20:07+00:00","remoteAddr":"127.0.0.1","user":"user2","app":"webdav","method":"DELETE","url":"\/owncloud\/remote.php\/dav\/files\/user2\/test","message":"Exception: HTTP\/1.1 423 Locked: {\"Exception\":\"Sabre\\\\DAV\\\\Exception\\\\Locked\",\"Message\":\"\",\"Code\":0,\"Trace\":\"#0 \\\/srv\\\/www\\\/htdocs\\\/owncloud\\\/lib\\\/composer\\\/sabre\\\/event\\\/lib\\\/WildcardEmitterTrait.php(96): Sabre\\\\DAV\\\\Locks\\\\Plugin->validateTokens(Object(Sabre\\\\HTTP\\\\Request), Array)\\n#1 \\\/srv\\\/www\\\/htdocs\\\/owncloud\\\/lib\\\/composer\\\/sabre\\\/dav\\\/lib\\\/DAV\\\/Server.php(1448): Sabre\\\\DAV\\\\Server->emit('validateTokens', Array)\\n#2 \\\/srv\\\/www\\\/htdocs\\\/owncloud\\\/lib\\\/composer\\\/sabre\\\/dav\\\/lib\\\/DAV\\\/Server.php(458): Sabre\\\\DAV\\\\Server->checkPreconditions(Object(Sabre\\\\HTTP\\\\Request), Object(Sabre\\\\HTTP\\\\Response))\\n#3 \\\/srv\\\/www\\\/htdocs\\\/owncloud\\\/lib\\\/composer\\\/sabre\\\/dav\\\/lib\\\/DAV\\\/Server.php(241): Sabre\\\\DAV\\\\Server->invokeMethod(Object(Sabre\\\\HTTP\\\\Request), Object(Sabre\\\\HTTP\\\\Response))\\n#4 \\\/srv\\\/www\\\/htdocs\\\/owncloud\\\/lib\\\/composer\\\/sabre\\\/dav\\\/lib\\\/DAV\\\/Server.php(309): Sabre\\\\DAV\\\\Server->start()\\n#5 \\\/srv\\\/www\\\/htdocs\\\/owncloud\\\/apps\\\/dav\\\/lib\\\/Server.php(299): Sabre\\\\DAV\\\\Server->exec()\\n#6 \\\/srv\\\/www\\\/htdocs\\\/owncloud\\\/apps\\\/dav\\\/appinfo\\\/v2\\\/remote.php(31): OCA\\\\DAV\\\\Server->exec()\\n#7 \\\/srv\\\/www\\\/htdocs\\\/owncloud\\\/remote.php(175): require_once('\\\/srv\\\/www\\\/htdocs...')\\n#8 {main}\",\"File\":\"\\\/srv\\\/www\\\/htdocs\\\/owncloud\\\/lib\\\/composer\\\/sabre\\\/dav\\\/lib\\\/DAV\\\/Locks\\\/Plugin.php\",\"Line\":518}"}

Version

Observed on #32250

In general for any locking operations, we need to consider it relative to the storage owner's storage.
If an operation would not affect the original folder like unsharing from self, we should let it through.

@DeepDiver1975 @individual-it FYI

@ownclouders
Copy link
Contributor

GitMate.io thinks possibly related issues are #28337 (Applications cannot detect "unshare from self"), #17244 (Unshare from user must lock path to avoid recipient access), #33778 (Persistent lock timeout value is "Second-" when not specified.), and #28197 (Old chunking on new endpoint broken when using if-match header).

@PVince81
Copy link
Contributor Author

  • reenable acceptance test after fixing

@PVince81
Copy link
Contributor Author

PVince81 commented Jan 9, 2019

might be related to #33885 which is fixed now.

will submit a PR to reenable the test which will tell us whether it works now...

@PVince81 PVince81 mentioned this issue Jan 9, 2019
10 tasks
@PVince81
Copy link
Contributor Author

PVince81 commented Jan 9, 2019

PR here #34099

@PVince81
Copy link
Contributor Author

failed... ok, then this will need more bugfixing

@PVince81
Copy link
Contributor Author

Expected behavior for now. Could reconsider in the future.

Action point: adjust acceptance test

@PVince81
Copy link
Contributor Author

@individual-it can you adjust the test expectation ?

@PVince81
Copy link
Contributor Author

the "expected for now" related to a discussion with @DeepDiver1975 where the idea was that we are "touching" the resource anyway, even if it doesn't affect the owner. Or say the resource is locked so people should not be able to do anything with it.

@lock lock bot locked as resolved and limited conversation to collaborators Jan 31, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants