Add isDeletable() check to OC\Files\Node::move()

[PhrozenByte]

Moving a file (or directory) is the same as creating a copy of the file and deleting the original one. This is also enforced on a filesystem level: One can't move 'foo/test' to 'bar/test' unless the user has 'wx' permissions on both 'foo' and 'bar'. In practice this never was an issue because the low-level storage returns 'false' anyway, but it might be in the future.

[ChristophWurst]

Moving a file (or directory) is the same as creating a copy of the file and deleting the original one

Is this really the case for all storage? What about object storage and similar?

[PhrozenByte]

Since there's no distinct MOVE permission in Nextcloud, but CREATE and DELETE permissions only, this is true for all storages (including object storages), yes. Otherwise we wouldn't check whether we can actually (re)move the original file/folder - the main difference between copy() and move(). Any storage should yield a low-level failure right now, if they don't this is a bug (if not a security issue).

[PhrozenByte]

@ChristophWurst Any thoughts?

[icewind1991]

Nextcloud itself only requires update permissions for moving files within a storage (for example, you can rename files within a share even if you don't have delete permissions).

I think that adding an isUpdateable check for the parent folder handles things correctly.

[PhrozenByte]

I think that adding an isUpdateable check for the parent folder handles things correctly.

From a storage's point of view (and the filesystem's) this is basically the same, so I'm fine with it:

server/lib/private/Files/Storage/Common.php

Lines 154 to 160 in 1daab57

	public function isDeletable($path) {
	if ($path === '' || $path === '/') {
	return $this->isUpdatable($path);
	}
	$parent = dirname($path);
	return $this->isUpdatable($parent) && $this->isUpdatable($path);
	}

However, this isn't going to work this easily, because the root folder isn't updateable per se. That's probably the reason why there's a separate creatable permission? Anyway, we could add an exception for the root folder, but I'm not sure whether this is feasible... Ideas?

[PhrozenByte]

I'd say that we have a per-design issue here that would require overhauling the file permissions system as a whole, what probably isn't worth it. Opinions?

[PhrozenByte]

Closing as of #27734 (comment)