add some additonal permission checks to the webdav backend#255
add some additonal permission checks to the webdav backend#255
Conversation
3182f86 to
93c8458
Compare
|
Thanks, @schiessle. I'll see if I can write an integration test for this. |
... or even better: let @schiessle try to add this ;) |
Ah. I guess he'll be busy fixing the theming PR 🙈 |
|
Integration tests passed. They should pass, but there are some broken unit tests, @schiessle mind taking a look at those? |
eae9b92 to
f43fba4
Compare
f43fba4 to
4e21543
Compare
| $sourcePermission = $infoSource && $infoSource->isDeletable(); | ||
|
|
||
| if (!$destinationPermission || !$sourcePermission) { | ||
| throw new Forbidden(); |
There was a problem hiding this comment.
Required parameter $message missing
|
Tests should pass now after ea18d86 |
| $info = $this->fileView->getFileInfo(dirname($destination)); | ||
| if ($info && !$info->isUpdateable()) { | ||
| throw new Forbidden(); | ||
| throw new Forbidden('No permissions to move object.'); |
There was a problem hiding this comment.
"copy" in this case? 😉
|
Tested and works 👍 |
|
👍 for the tests |
|
|
||
|
|
||
| $info = $this->fileView->getFileInfo(dirname($destination)); | ||
| if ($info && !$info->isUpdateable()) { |
There was a problem hiding this comment.
Might not be 100% correct. A copy needs create permissions for a "copy in" as new file.
As for "copy + overwrite" it's "create + delete", because SabreDAV will trigger an automatic delete of the target before running this method. So in the end only a check on "create" is needed.
Even though the "update" permission semantically seems to fit better for overwrites, it never quite worked as expected.
@LukasReschke another one for you 😉